- 08 Nov, 2018 18 commits
-
-
Jan Kara authored
The nested acquisition of loop_ctl_mutex (->lo_ctl_mutex back then) has been introduced by commit f028f3b2 "loop: fix circular locking in loop_clr_fd()" to fix lockdep complains about bd_mutex being acquired after lo_ctl_mutex during partition rereading. Now that these are properly fixed, let's stop fooling lockdep. Signed-off-by:
Jan Kara <jack@suse.cz> Signed-off-by:
Jens Axboe <axboe@kernel.dk>
-
Jan Kara authored
Code in loop_change_fd() drops reference to the old file (and also the new file in a failure case) under loop_ctl_mutex. Similarly to a situation in loop_set_fd() this can create a circular locking dependency if this was the last reference holding the file open. Delay dropping of the file reference until we have released loop_ctl_mutex. Reported-by:
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> Signed-off-by:
-
Jan Kara authored
Calling blkdev_reread_part() under loop_ctl_mutex causes lockdep to complain about circular lock dependency between bdev->bd_mutex and lo->lo_ctl_mutex. The problem is that on loop device open or close lo_open() and lo_release() get called with bdev->bd_mutex held and they need to acquire loop_ctl_mutex. OTOH when loop_reread_partitions() is called with loop_ctl_mutex held, it will call blkdev_reread_part() which acquires bdev->bd_mutex. See syzbot report for details [1]. Move call to blkdev_reread_part() in __loop_clr_fd() from under loop_ctl_mutex to finish fixing of the lockdep warning and the possible deadlock. [1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588Reported-by:
syzbot <syzbot+4684a000d5abdade83fac55b1e7d1f935ef1936e@syzkaller.appspotmail.com> Signed-off-by:
-
Jan Kara authored
Calling loop_reread_partitions() under loop_ctl_mutex causes lockdep to complain about circular lock dependency between bdev->bd_mutex and lo->lo_ctl_mutex. The problem is that on loop device open or close lo_open() and lo_release() get called with bdev->bd_mutex held and they need to acquire loop_ctl_mutex. OTOH when loop_reread_partitions() is called with loop_ctl_mutex held, it will call blkdev_reread_part() which acquires bdev->bd_mutex. See syzbot report for details [1]. Move all calls of loop_rescan_partitions() out of loop_ctl_mutex to avoid lockdep warning and fix deadlock possibility. [1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588Reported-by:
-
Jan Kara authored
The call of __blkdev_reread_part() from loop_reread_partition() happens only when we need to invalidate partitions from loop_release(). Thus move a detection for this into loop_clr_fd() and simplify loop_reread_partition(). This makes loop_reread_partition() safe to use without loop_ctl_mutex because we use only lo->lo_number and lo->lo_file_name in case of error for reporting purposes (thus possibly reporting outdate information is not a big deal) and we are safe from 'lo' going away under us by elevated lo->lo_refcnt. Signed-off-by:
-
Jan Kara authored
Push loop_ctl_mutex down to loop_change_fd(). We will need this to be able to call loop_reread_partitions() without loop_ctl_mutex. Signed-off-by:
-
Jan Kara authored
Push lo_ctl_mutex down to loop_set_fd(). We will need this to be able to call loop_reread_partitions() without lo_ctl_mutex. Signed-off-by:
-
Jan Kara authored
Push loop_ctl_mutex down to loop_set_status(). We will need this to be able to call loop_reread_partitions() without loop_ctl_mutex. Signed-off-by:
-
Jan Kara authored
Push loop_ctl_mutex down to loop_get_status() to avoid the unusual convention that the function gets called with loop_ctl_mutex held and releases it. Signed-off-by:
-
Jan Kara authored
loop_clr_fd() has a weird locking convention that is expects loop_ctl_mutex held, releases it on success and keeps it on failure. Untangle the mess by moving locking of loop_ctl_mutex into loop_clr_fd(). Signed-off-by:
-
Jan Kara authored
Move setting of lo_state to Lo_rundown out into the callers. That will allow us to unlock loop_ctl_mutex while the loop device is protected from other changes by its special state. Signed-off-by:
-
Jan Kara authored
Push acquisition of lo_ctl_mutex down into individual ioctl handling branches. This is a preparatory step for pushing the lock down into individual ioctl handling functions so that they can release the lock as they need it. We also factor out some simple ioctl handlers that will not need any special handling to reduce unnecessary code duplication. Signed-off-by:
-
Jan Kara authored
Now that loop_ctl_mutex is global, just get rid of loop_index_mutex as there is no good reason to keep these two separate and it just complicates the locking. Signed-off-by:
-
Jan Kara authored
__loop_release() has a single call site. Fold it there. This is currently not a huge win but it will make following replacement of loop_index_mutex more obvious. Signed-off-by:
-
Tetsuo Handa authored
syzbot is reporting NULL pointer dereference [1] which is caused by race condition between ioctl(loop_fd, LOOP_CLR_FD, 0) versus ioctl(other_loop_fd, LOOP_SET_FD, loop_fd) due to traversing other loop devices at loop_validate_file() without holding corresponding lo->lo_ctl_mutex locks. Since ioctl() request on loop devices is not frequent operation, we don't need fine grained locking. Let's use global lock in order to allow safe traversal at loop_validate_file(). Note that syzbot is also reporting circular locking dependency between bdev->bd_mutex and lo->lo_ctl_mutex [2] which is caused by calling blkdev_reread_part() with lock held. This patch does not address it. [1] https://syzkaller.appspot.com/bug?id=f3cfe26e785d85f9ee259f385515291d21bd80a3 [2] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d15889Signed-off-by:
syzbot <syzbot+bf89c128e05dd6c62523@syzkaller.appspotmail.com> Reviewed-by:
-
Tetsuo Handa authored
vfs_getattr() needs "struct path" rather than "struct file". Let's use path_get()/path_put() rather than get_file()/fput(). Signed-off-by:
-
Colin Ian King authored
Pointer 'set' is declared but not used, remove it. Cleans up warning: warning: unused variable ‘set’ [-Wunused-variable] Signed-off-by:
Colin Ian King <colin.king@canonical.com> Signed-off-by:
-
Jens Axboe authored
Stephen reports: After merging the block tree, today's linux-next build (sparc64 defconfig) produced this warning: /home/sfr/next/next/drivers/block/sunvdc.c: In function 'init_queue': /home/sfr/next/next/drivers/block/sunvdc.c:788:6: warning: unused variable 'ret' [-Wunused-variable] int ret; ^~~ Kill the unused variable. Fixes: fa182a1f ("sunvdc: convert to blk-mq") Reported-by:Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by:
-
- 07 Nov, 2018 22 commits
-
-
Jens Axboe authored
Adds support for defining a variable number of poll queues, currently configurable with the 'poll_queues' module parameter. Defaults to a single poll queue. And now we finally have poll support without triggering interrupts! Reviewed-by:
Hannes Reinecke <hare@suse.com> Reviewed-by:
Keith Busch <keith.busch@intel.com> Signed-off-by:
-
Jens Axboe authored
We use IOCB_HIPRI to poll for IO in the caller instead of scheduling. This information is not available for (or after) IO submission. The driver may make different queue choices based on the type of IO, so make the fact that we will poll for this IO known to the lower layers as well. Reviewed-by:
Sagi Grimberg <sagi@grimberg.me> Signed-off-by:
-
Jens Axboe authored
NVMe does round-robin between queues by default, which means that sharing a queue map for both reads and writes can be problematic in terms of read servicing. It's much easier to flood the queue with writes and reduce the read servicing. Implement two queue maps, one for reads and one for writes. The write queue count is configurable through the 'write_queues' parameter. By default, we retain the previous behavior of having a single queue set, shared between reads and writes. Setting 'write_queues' to a non-zero value will create two queue sets, one for reads and one for writes, the latter using the configurable number of queues (hardware queue counts permitting). Reviewed-by:
-
Jens Axboe authored
Add a queue offset to the tag map. This enables users to map iteratively, for each queue map type they support. Bump maximum number of supported maps to 2, we're now fully able to support more than 1 map. Reviewed-by:
-
Jens Axboe authored
Currently we only look at the software queue, but with support for multiple maps, we should also look at the hardware queue. This is important since we'll flush out the request list if either the software queue or hardware queue don't match. This sorts by software queue first, then hardware queue if that differs. Finally we sort by request location like before. This minimizes the flush points per plug list. Reviewed-by:
-
Jens Axboe authored
It's somewhat strange to have a list insertion function that relies on the fact that the caller has mapped things correctly. Pass in the hardware queue directly for insertion, which makes for a much cleaner interface and implementation. Reviewed-by:
-
Jens Axboe authored
We call blk_mq_map_queue() a lot, at least two times for each request per IO, sometimes more. Since we now have an indirect call as well in that function. cache the mapping so we don't have to re-call blk_mq_map_queue() for the same request multiple times. Reviewed-by:
-
Jens Axboe authored
With multiple maps, nr_cpu_ids is no longer the maximum number of hardware queues we support on a given devices. The initializer of the tag_set can have set ->nr_hw_queues larger than the available number of CPUs, since we can exceed that with multiple queue maps. Reviewed-by:
Bart Van Assche <bvanassche@acm.org> Reviewed-by:
-
Jens Axboe authored
Add support for the tag set carrying multiple queue maps, and for the driver to inform blk-mq how many it wishes to support through setting set->nr_maps. This adds an mq_ops helper for drivers that support more than 1 map, mq_ops->rq_flags_to_type(). The function takes request/bio flags and CPU, and returns a queue map index for that. We then use the type information in blk_mq_map_queue() to index the map set. Reviewed-by:
-
Jens Axboe authored
It can be useful for a user to verify what type a given hardware queue is, expose this information in sysfs. Reviewed-by:
-
Jens Axboe authored
The mapping used to be dependent on just the CPU location, but now it's a tuple of (type, cpu) instead. This is a prep patch for allowing a single software queue to map to multiple hardware queues. No functional changes in this patch. This changes the software queue count to an unsigned short to save a bit of space. We can still support 64K-1 CPUs, which should be enough. Add a check to catch a wrap. Reviewed-by:
-
Jens Axboe authored
Prep patch for being able to place request based not just on CPU location, but also on the type of request. Reviewed-by:
-
Jens Axboe authored
Doesn't do anything right now, but it's needed as a prep patch to get the interfaces right. While in there, correct the blk_mq_map_queue() CPU type to an unsigned int. Reviewed-by:
-
Jens Axboe authored
This is in preparation for allowing multiple sets of maps per queue, if so desired. Reviewed-by:
-
Jens Axboe authored
It's just a pointer to set->mq_map, use that instead. Move the assignment a bit earlier, so we always know it's valid. Reviewed-by:
Christoph Hellwig <hch@lst.de> Reviewed-by:
-
Jens Axboe authored
Merge branch 'irq/for-block' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip into for-4.21/block Pull in the irq affinity commits, that are staged through Thomas's tree. * 'irq/for-block' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: genirq/affinity: Add support for allocating interrupt sets genirq/affinity: Pass first vector to __irq_build_affinity_masks() genirq/affinity: Move two stage affinity spreading into a helper function genirq/affinity: Spread IRQs to all available NUMA nodes
-
Jens Axboe authored
This was used for completion placement for the legacy path, but for mq we have rq->mq_ctx->cpu for that. Add a helper to get the request CPU assignment, as the mq_ctx type is private to blk-mq. Reviewed-by:
Omar Sandoval <osandov@fb.com> Signed-off-by:
-
Jens Axboe authored
With the legacy path gone, all we do is funnel it through the mq_ops->complete() operation. Tested-by:
Ming Lei <ming.lei@redhat.com> Reviewed-by:
-
Jens Axboe authored
No point in hiding what this does, just open code it in the one spot where we are still using it. Reviewed-by:
-
Jens Axboe authored
This is dead code, any queue reaching this part has mq_ops attached. Reviewed-by:
-
Jens Axboe authored
It'll always be false at this point, just remove it. Reviewed-by:
-
Jens Axboe authored
Reviewed-by:
-
