Re: [PATCH] queue stall with blk-mq-sched

2017-01-24 Thread Hannes Reinecke
On 01/24/2017 11:06 PM, Jens Axboe wrote: > On 01/24/2017 12:55 PM, Jens Axboe wrote: >> Try this patch. We only want to bump it for the driver tags, not the >> scheduler side. > > More complete version, this one actually tested. I think this should fix > your issue, let me know. > Nearly there.

[PATCH] blkcg: fix double free of new_blkg in blkcg_init_queue

2017-01-24 Thread Hou Tao
if blkg_create fails, new_blkg passed as an argument will be freed by blkg_create, so there is no need to free it again. Signed-off-by: Hou Tao --- block/blk-cgroup.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index

Re: [LSF/MM TOPIC] block level event logging for storage media management

2017-01-24 Thread Song Liu
> On Jan 24, 2017, at 12:18 PM, Oleg Drokin wrote: > > > On Jan 23, 2017, at 2:27 AM, Dan Williams wrote: > >> [ adding Oleg ] >> >> On Sun, Jan 22, 2017 at 10:00 PM, Song Liu wrote: >>> Hi Dan, >>> >>> I think the the block level event log is

Re: [PATCH] queue stall with blk-mq-sched

2017-01-24 Thread Jens Axboe
On 01/24/2017 12:55 PM, Jens Axboe wrote: > Try this patch. We only want to bump it for the driver tags, not the > scheduler side. More complete version, this one actually tested. I think this should fix your issue, let me know. diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index

Re: [LSF/MM TOPIC] block level event logging for storage media management

2017-01-24 Thread Oleg Drokin
On Jan 23, 2017, at 2:27 AM, Dan Williams wrote: > [ adding Oleg ] > > On Sun, Jan 22, 2017 at 10:00 PM, Song Liu wrote: >> Hi Dan, >> >> I think the the block level event log is more like log only system. When en >> event >> happens, it is not necessary to take

Re: [Lsf-pc] [LSF/MM TOPIC] Badblocks checking/representation in filesystems

2017-01-24 Thread Vishal Verma
On 01/24, Jan Kara wrote: > On Fri 20-01-17 07:42:09, Dan Williams wrote: > > On Fri, Jan 20, 2017 at 1:47 AM, Jan Kara wrote: > > > On Thu 19-01-17 14:17:19, Vishal Verma wrote: > > >> On 01/18, Jan Kara wrote: > > >> > On Tue 17-01-17 15:37:05, Vishal Verma wrote: > > >> > 2) PMEM

Re: [PATCH] queue stall with blk-mq-sched

2017-01-24 Thread Jens Axboe
On 01/24/2017 11:49 AM, Hannes Reinecke wrote: > On 01/24/2017 05:09 PM, Jens Axboe wrote: >> On 01/24/2017 08:54 AM, Hannes Reinecke wrote: >>> Hi Jens, >>> >>> I'm trying to debug a queue stall with your blk-mq-sched branch; with my >>> latest mpt3sas patches fio stops basically directly after

Re: [PATCH 05/16] dm: always defer request allocation to the owner of the request_queue

2017-01-24 Thread Christoph Hellwig
On Tue, Jan 24, 2017 at 11:39:51AM -0500, Mike Snitzer wrote: > Fair enough. Cc'ing Junichi just in case he sees anything we're > missing. Thanks, I'll wait for his repsonse before reposting with the few accumulated changes (including the dm cleanup). -- To unsubscribe from this list: send the

Re: [PATCH] queue stall with blk-mq-sched

2017-01-24 Thread Hannes Reinecke
On 01/24/2017 05:09 PM, Jens Axboe wrote: On 01/24/2017 08:54 AM, Hannes Reinecke wrote: Hi Jens, I'm trying to debug a queue stall with your blk-mq-sched branch; with my latest mpt3sas patches fio stops basically directly after starting a sequential read :-( I've debugged things and came up

Re: [PATCH] queue stall with blk-mq-sched

2017-01-24 Thread Hannes Reinecke
On 01/24/2017 05:03 PM, Jens Axboe wrote: On 01/24/2017 08:54 AM, Hannes Reinecke wrote: Hi Jens, I'm trying to debug a queue stall with your blk-mq-sched branch; with my latest mpt3sas patches fio stops basically directly after starting a sequential read :-( I've debugged things and came up

Re: [PATCH 05/16] dm: always defer request allocation to the owner of the request_queue

2017-01-24 Thread Mike Snitzer
On Tue, Jan 24 2017 at 9:20am -0500, Christoph Hellwig wrote: > On Tue, Jan 24, 2017 at 05:05:39AM -0500, Mike Snitzer wrote: > > possible and is welcomed cleanup. The only concern I have is that using > > get_request() for the old request_fn request_queue eliminates the > >

Re: [PATCH 15/16] block: split scsi_request out of struct request

2017-01-24 Thread Bart Van Assche
On Tue, 2017-01-24 at 09:09 +0100, h...@lst.de wrote: > On Tue, Jan 24, 2017 at 12:33:13AM +, Bart Van Assche wrote: > > Do we perhaps need a check before the above memcpy() call whether or not > > sense == NULL? > > Yes, probably. I didn't think of the case where the caller wouldn't > pass

Re: [PATCH] queue stall with blk-mq-sched

2017-01-24 Thread Jens Axboe
On 01/24/2017 08:54 AM, Hannes Reinecke wrote: > Hi Jens, > > I'm trying to debug a queue stall with your blk-mq-sched branch; with my > latest mpt3sas patches fio stops basically directly after starting a > sequential read :-( > > I've debugged things and came up with the attached patch; we

Re: [PATCH] queue stall with blk-mq-sched

2017-01-24 Thread Jens Axboe
On 01/24/2017 08:54 AM, Hannes Reinecke wrote: > Hi Jens, > > I'm trying to debug a queue stall with your blk-mq-sched branch; with my > latest mpt3sas patches fio stops basically directly after starting a > sequential read :-( > > I've debugged things and came up with the attached patch; we

[PATCH] queue stall with blk-mq-sched

2017-01-24 Thread Hannes Reinecke
Hi Jens, I'm trying to debug a queue stall with your blk-mq-sched branch; with my latest mpt3sas patches fio stops basically directly after starting a sequential read :-( I've debugged things and came up with the attached patch; we need to restart waiters with blk_mq_tag_idle() after completing

Re: [PATCH 01/16] block: fix elevator init check

2017-01-24 Thread Christoph Hellwig
On Tue, Jan 24, 2017 at 08:06:39AM -0700, Jens Axboe wrote: > We check for REQ_PREFLUSH | REQ_FUA in so many places though, might not > be a bad idea to keep the helper but just make it take the opf and fix > up the other locations too. The fact that PREFLUSH|FUA is the magic to > check for is

Re: [PATCH 01/16] block: fix elevator init check

2017-01-24 Thread Jens Axboe
On Mon, Jan 23 2017, Christoph Hellwig wrote: > We can't initalize the elevator fields for flushes as flush share space > in struct request with the elevator data. But currently we can't > commnicate that a request is a flush through blk_get_request as we > can only pass READ or WRITE, and the

Re: [PATCH] blk-mq-sched: fix possible crash if changing scheduler fails

2017-01-24 Thread Jens Axboe
On 01/23/2017 04:47 PM, Omar Sandoval wrote: > From: Omar Sandoval > > In elevator_switch(), we free the old scheduler's tags and then > initialize the new scheduler. If initializing the new scheduler fails, > we fall back to the old scheduler, but our tags have already been

Re: [PATCH] block: fix use after free in __blkdev_direct_IO

2017-01-24 Thread Jens Axboe
On 01/24/2017 06:50 AM, Christoph Hellwig wrote: > We can't dereference the dio structure after submitting the last bio for > this request, as I/O completion might have happened before the code is > run. Introduce a local is_sync variable instead. Applied. -- Jens Axboe -- To unsubscribe from

Re: [PATCH 05/16] dm: always defer request allocation to the owner of the request_queue

2017-01-24 Thread Christoph Hellwig
On Tue, Jan 24, 2017 at 05:05:39AM -0500, Mike Snitzer wrote: > possible and is welcomed cleanup. The only concern I have is that using > get_request() for the old request_fn request_queue eliminates the > guaranteed availability of requests to allow for forward progress (on > path failure or for

[PATCH] block: fix use after free in __blkdev_direct_IO

2017-01-24 Thread Christoph Hellwig
We can't dereference the dio structure after submitting the last bio for this request, as I/O completion might have happened before the code is run. Introduce a local is_sync variable instead. Fixes: 542ff7bf ("block: new direct I/O implementation") Signed-off-by: Christoph Hellwig

Re: BUG: KASAN: Use-after-free

2017-01-24 Thread Matias Bjørling
On 01/24/2017 02:34 PM, Christoph Hellwig wrote: > On Tue, Jan 24, 2017 at 11:01:42AM +0100, Matias Bjørling wrote: >> Yup. That fixes it. Should I put together the patch, or will you take >> care of it? > > I'll send it out. Of course with proper reporting credits for you. > Awesome. Thanks

Re: BUG: KASAN: Use-after-free

2017-01-24 Thread Christoph Hellwig
On Tue, Jan 24, 2017 at 11:01:42AM +0100, Matias Bjørling wrote: > Yup. That fixes it. Should I put together the patch, or will you take > care of it? I'll send it out. Of course with proper reporting credits for you. -- To unsubscribe from this list: send the line "unsubscribe linux-block" in

Re: [PATCH 10/10] blk-mq: move hctx and ctx counters from sysfs to debugfs

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > These counters aren't as out-of-place in sysfs as the other stuff, but > debugfs is a slightly better home for them. > > Signed-off-by: Omar Sandoval > --- > block/blk-mq-debugfs.c | 181 >

Re: [PATCH 09/10] blk-mq: move hctx io_poll, stats, and dispatched from sysfs to debugfs

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > These statistics _might_ be useful to userspace, but it's better not to > commit to an ABI for these yet. Also, the dispatched file in sysfs > couldn't be cleared, so make it clearable like the others in

Re: [PATCH 08/10] blk-mq: add tags and sched_tags bitmaps to debugfs

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > These can be used to debug issues like tag leaks and stuck requests. > > Signed-off-by: Omar Sandoval > --- > block/blk-mq-debugfs.c | 50 >

Re: [PATCH 07/10] blk-mq: move tags and sched_tags info from sysfs to debugfs

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > These are very tied to the blk-mq tag implementation, so exposing them > to sysfs isn't a great idea. Move the debugging information to debugfs > and add basic entries for the number of tags and the number of

Re: [PATCH 05/10] sbitmap: add helpers for dumping to a seq_file

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > This is useful debugging information that will be used in the blk-mq > debugfs directory. > > Signed-off-by: Omar Sandoval > --- > include/linux/sbitmap.h | 34 >

Re: [PATCH 06/10] blk-mq: export software queue pending map to debugfs

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > This is useful for debugging problems where we've gotten stuck with > requests in the software queues. > > Signed-off-by: Omar Sandoval > --- > block/blk-mq-debugfs.c | 15 +++ >

Re: [PATCH 04/10] blk-mq: add extra request information to debugfs

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > The request pointers by themselves aren't super useful. > > Signed-off-by: Omar Sandoval > --- > block/blk-mq-debugfs.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff

Re: [PATCH 03/10] blk-mq: move hctx->dispatch and ctx->rq_list from sysfs to debugfs

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > These lists are only useful for debugging; they definitely don't belong > in sysfs. Putting them in debugfs also removes the limitation of a > single page of output. > > Signed-off-by: Omar Sandoval

Re: [PATCH 02/10] blk-mq: add hctx->{state,flags} to debugfs

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > hctx->state could come in handy for bugs where the hardware queue gets > stuck in the stopped state, and hctx->flags is just useful to know. > > Signed-off-by: Omar Sandoval > --- >

Re: [PATCH 01/10] blk-mq: create debugfs directory tree

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 07:59 PM, Omar Sandoval wrote: > From: Omar Sandoval > > In preparation for putting blk-mq debugging information in debugfs, > create a directory tree mirroring the one in sysfs: > > # tree -d /sys/kernel/debug/block > /sys/kernel/debug/block > |--

Re: [PATCH 16/16] block: don't assign cmd_flags in __blk_rq_prep_clone

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 04:29 PM, Christoph Hellwig wrote: > These days we have the proper flags set since request allocation time. > > Signed-off-by: Christoph Hellwig > --- > block/blk-core.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/block/blk-core.c b/block/blk-core.c >

Re: [PATCH 15/16] block: split scsi_request out of struct request

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 04:29 PM, Christoph Hellwig wrote: > And require all drivers that want to support BLOCK_PC to allocate it > as the first thing of their private data. To support this the legacy > IDE and BSG code is switched to set cmd_size on their queues to let > the block layer allocate the

Re: [PATCH 13/16] scsi: allocate scsi_cmnd structures as part of struct request

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 04:29 PM, Christoph Hellwig wrote: > Rely on the new block layer functionality to allocate additional driver > specific data behind struct request instead of implementing it in SCSI > itѕelf. > > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/hosts.c | 20

Re: [PATCH 09/16] scsi: remove gfp_flags member in scsi_host_cmd_pool

2017-01-24 Thread Johannes Thumshirn
On Mon, Jan 23, 2017 at 04:29:14PM +0100, Christoph Hellwig wrote: > When using the slab allocator we already decide at cache creation time if > an allocation comes from a GFP_DMA pool using the SLAB_CACHE_DMA flag, > and there is no point passing the kmalloc-family only GFP_DMA flag to >

[PATCH] lightnvm: use end_io callback instead of instance

2017-01-24 Thread Matias Bjørling
When the lightnvm core had the "gennvm" layer between the device and the target, there was a need for the core to be able to figure out which target it should send an end_io callback to. Leading to a "double" end_io, first for the media manager instance, and then for the target instance. Now that

Re: [PATCH 12/16] scsi: remove __scsi_alloc_queue

2017-01-24 Thread Johannes Thumshirn
On Mon, Jan 23, 2017 at 04:29:17PM +0100, Christoph Hellwig wrote: > Instead do an internal export of __scsi_init_queue for the transport > classes that export BSG nodes. > > Signed-off-by: Christoph Hellwig > --- Looks good, Reviewed-by: Johannes Thumshirn --

Re: [PATCH 12/16] scsi: remove __scsi_alloc_queue

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 04:29 PM, Christoph Hellwig wrote: > Instead do an internal export of __scsi_init_queue for the transport > classes that export BSG nodes. > > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/scsi_lib.c | 19 --- >

Re: [PATCH 11/16] scsi: remove scsi_cmd_dma_pool

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 04:29 PM, Christoph Hellwig wrote: > There is no need for GFP_DMA allocations of the scsi_cmnd structures > themselves, all that might be DMAed to or from is the actual payload, > or the sense buffers. > > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/scsi.c |

Re: [PATCH 10/16] scsi: respect unchecked_isa_dma for blk-mq

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 04:29 PM, Christoph Hellwig wrote: > Currently blk-mq always allocates the sense buffer using normal GFP_KERNEL > allocation. Refactor the cmd pool code to split the cmd and sense allocation > and share the code to allocate the sense buffers as well as the sense buffer > slab caches

[PATCH 6/6] mmc: block: stop passing around pointless return values

2017-01-24 Thread Linus Walleij
The mmc_blk_issue_rq() function is called in exactly one place in queue.c and there the return value is ignored. So the functions called from that function that also meticulously return 0/1 do so for no good reason. Error reporting on the asynchronous requests are done upward to the block layer

[PATCH 4/6] mmc: block: inline command abortions

2017-01-24 Thread Linus Walleij
Setting rqc to NULL followed by a goto to cmd_abort is just a way to do unconditional abort without starting any new command. Inline the calls to mmc_blk_rw_cmd_abort() and return immediately in those cases. As a result, mmc_blk_rw_start_new() is not called with NULL requests, and we can remove

[PATCH 3/6] mmc: block: do not assign mq_rq when aborting command

2017-01-24 Thread Linus Walleij
The code in mmc_blk_issue_rq_rq() aborts a command if the request is not properly aligned on large sectors. As part of the path jumping out, it assigns the local variable mq_rq reflecting a MMC queue request to the current MMC queue request, which is confusing since the variable is not used after

[PATCH 2/6] mmc: block: break out mmc_blk_rw_start_new()

2017-01-24 Thread Linus Walleij
As a step toward breaking apart the very complex function mmc_blk_issue_rw_rq() we break out the code to start a new request. Signed-off-by: Linus Walleij --- drivers/mmc/core/block.c | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-)

[PATCH 0/6] mmc: block: command issue cleanups

2017-01-24 Thread Linus Walleij
The function mmc_blk_issue_rw_rq() is hopelessly convoluted and need to be refactored to it can be understood by humans. In the process I found some weird magic return values passed around for no good reason. Things are more readable after this. This work is done towards the goal of breaking

[PATCH 1/6] mmc: block: break out mmc_blk_rw_cmd_abort()

2017-01-24 Thread Linus Walleij
As a first step toward breaking apart the very complex function mmc_blk_issue_rw_rq() we break out the command abort code. This code assumes "ret" is != 0 and then repeatedly hammers blk_end_request() until the request to the block layer to end the request succeeds. Signed-off-by: Linus Walleij

Re: [PATCH 05/16] dm: always defer request allocation to the owner of the request_queue

2017-01-24 Thread Mike Snitzer
On Mon, Jan 23 2017 at 10:29am -0500, Christoph Hellwig wrote: > DM already calls blk_mq_alloc_request on the request_queue of the > underlying device if it is a blk-mq device. But now that we allow drivers > to allocate additional data and initialize it ahead of time we need to do

Re: BUG: KASAN: Use-after-free

2017-01-24 Thread Matias Bjørling
On 01/24/2017 10:52 AM, Christoph Hellwig wrote: > On Tue, Jan 24, 2017 at 10:32:11AM +0100, Matias Bjørling wrote: >> *(gdb) list *blkdev_direct_IO+0x50c >> 0x8142ab8c is in blkdev_direct_IO (fs/block_dev.c:401). >> 396 submit_bio(bio); >> 397 bio =

Re: BUG: KASAN: Use-after-free

2017-01-24 Thread Christoph Hellwig
On Tue, Jan 24, 2017 at 10:32:11AM +0100, Matias Bjørling wrote: > *(gdb) list *blkdev_direct_IO+0x50c > 0x8142ab8c is in blkdev_direct_IO (fs/block_dev.c:401). > 396 submit_bio(bio); > 397 bio = bio_alloc(GFP_KERNEL, nr_pages); > 398 } > 399

Re: BUG: KASAN: Use-after-free

2017-01-24 Thread Matias Bjørling
On 01/23/2017 06:20 PM, Christoph Hellwig wrote: > On Mon, Jan 23, 2017 at 05:07:52PM +0100, Matias Bjørling wrote: >> Hi, >> >> I could use some help verifying an use-after-free bug that I am seeing >> after the new direct I/O work went in. >> >> When issuing a direct write io using libaio, a bio

Re: [PATCH 05/16] dm: always defer request allocation to the owner of the request_queue

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 04:29 PM, Christoph Hellwig wrote: > DM already calls blk_mq_alloc_request on the request_queue of the > underlying device if it is a blk-mq device. But now that we allow drivers > to allocate additional data and initialize it ahead of time we need to do > the same for all drivers.

Re: [PATCH 04/16] dm: remove incomple BLOCK_PC support

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 04:29 PM, Christoph Hellwig wrote: > DM tries to copy a few fields around for BLOCK_PC requests, but given > that no dm-target ever wires up scsi_cmd_ioctl BLOCK_PC can't actually > be sent to dm. > > Signed-off-by: Christoph Hellwig > --- > drivers/md/dm-rq.c | 16

Re: [PATCH 03/16] block: allow specifying size for extra command data

2017-01-24 Thread Hannes Reinecke
On 01/23/2017 04:29 PM, Christoph Hellwig wrote: > This mirrors the blk-mq capabilities to allocate extra drivers-specific > data behind struct request by setting a cmd_size field, as well as having > a constructor / destructor for it. > > Signed-off-by: Christoph Hellwig > --- >