On Wed, May 16, 2018 at 04:17:42PM +, Bart Van Assche wrote:
> There is another reason the deadline is included in the atomic operation,
> namely to handle races between the BLK_EH_RESET_TIMER case in
> blk_mq_rq_timed_out()
> and blk_mq_complete_request(). I don't think that race is addressed
On Wed, May 16, 2018 at 04:47:54PM +, Bart Van Assche wrote:
> I think your patch changes the order of changing the request state and
> calling mod_timer(). In my patch the request state and the deadline are
> updated first and mod_timer() is called afterwards. I think your patch
> changes the
On Fri, Jul 13, 2018 at 05:58:08PM -0600, Keith Busch wrote:
> Of the two you mentioned, yours is preferable IMO. While I appreciate
> Jianchao's detailed analysis, it's hard to take a proposal seriously
> that so colourfully calls everyone else "dangerous" while advocating
> for silently losing re
On Wed, Jul 18, 2018 at 09:56:50PM +0200, h...@lst.de wrote:
> On Fri, Jul 13, 2018 at 05:58:08PM -0600, Keith Busch wrote:
> > Of the two you mentioned, yours is preferable IMO. While I appreciate
> > Jianchao's detailed analysis, it's hard to take a proposal seriousl
On Wed, Jul 18, 2018 at 03:17:11PM -0600, Keith Busch wrote:
> And there may be other drivers that don't want their completions
> ignored, so breaking them again is also a step in the wrong direction.
>
> There are not that many blk-mq drivers, so we can go through them all.
I think the point is
On Thu, Jul 19, 2018 at 10:22:40AM -0600, Keith Busch wrote:
> On Thu, Jul 19, 2018 at 04:04:46PM +, Bart Van Assche wrote:
> > On Thu, 2018-07-19 at 09:56 -0600, Keith Busch wrote:
> > > Even some scsi drivers are susceptible to losing their requests with the
> > > reverted behavior: take virt
On Mon, Aug 06, 2018 at 01:54:40PM +, Bart Van Assche wrote:
> On Mon, 2018-08-06 at 08:27 +0200, Hannes Reinecke wrote:
> > I am not sure this is going to work for SCSI parallel; we're using the
> > QUIESCE state there to do domain validation, and all commands there are
> > most definitely not
On Thu, Mar 30, 2017 at 11:29:29AM -0400, Martin K. Petersen wrote:
> "h...@lst.de" writes:
>
> > Jens, any opinion? I'd like to remove it too, but I fear it might
> > break things. We could deprecate it first with a warning when read
> > and then remove
On Thu, Mar 30, 2017 at 11:28:32AM -0400, Martin K. Petersen wrote:
> "h...@lst.de" writes:
>
> Christoph,
>
> > On Tue, Mar 28, 2017 at 04:48:55PM +, Bart Van Assche wrote:
> >> > if (sdp->no_write_same)
> >> >
On Thu, Mar 30, 2017 at 10:19:55PM -0400, Martin K. Petersen wrote:
> > If you manually change the provisioning mode to WS10 on a device that
> > must use WRITE SAME (16) to be able to address all blocks you're already
> > screwed right now, and with this patch you can screw yourself through
> > th
On Thu, Apr 13, 2017 at 07:58:13PM +, Bart Van Assche wrote:
> Should the person who submitted this driver be CC-ed for this patch (unsik
> Kim )?
Yes, he should. And in fact he was when I sent this patch out separately
a little earlier, I just included it in this series for reference.
On Thu, Apr 13, 2017 at 08:03:22PM +, Bart Van Assche wrote:
> That blk_execute_rq() call can only be reached if a few lines above 0 was
> assigned to the "error" variable. Since nfsd4_scsi_identify_device() returns
> the value of the "error" variable I think -EIO should be assigned to that
> v
On Mon, Apr 17, 2017 at 10:01:09AM -0600, Jens Axboe wrote:
> Are you respinning this series for 4.12?
Yes, I think I got enough feedback by now to resend it. I'll try to
get it out today.
On Tue, Apr 18, 2017 at 10:24:15PM +, Bart Van Assche wrote:
> On Tue, 2017-04-18 at 08:52 -0700, Christoph Hellwig wrote:
> > diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
> > index b05e151c9b38..82c6d02193ae 100644
> > --- a/drivers/block/paride/pd.c
> > +++ b/drivers/blo
On Tue, Apr 18, 2017 at 10:34:20PM +, Bart Van Assche wrote:
> Did you perhaps intend "req->result" instead of "rq->result"?
Yes.
> Did you intend "war" or is that perhaps a typo?
I'll fix the comment.
> > trace_scsi_dispatch_cmd_done(cmd);
> > - blk_mq_complete_request(cmd->request,
On Tue, Apr 18, 2017 at 10:57:11PM +, Bart Van Assche wrote:
> Both blk-mq and the traditional block layer support a .cmd_size field to
> make the block layer core allocate driver-specific data at the end of struct
> request. Could that mechanism have been used for the error_count field?
It co
On Tue, Apr 18, 2017 at 10:50:18PM +, Bart Van Assche wrote:
> On Tue, 2017-04-18 at 08:52 -0700, Christoph Hellwig wrote:
> > Now that we always have a ->complete callback we can remove the direct
> > call to blk_mq_end_request, as well as the error argument to
> > blk_mq_complete_request.
>
On Wed, Apr 19, 2017 at 09:07:45PM +, Bart Van Assche wrote:
> > + blk_execute_rq(or->request->q, NULL, or->request, 0);
> > + error = or->request ? -EIO : 0;
>
> Hello Christoph,
>
> Did you perhaps intend or->request->errors instead of or->request?
Yes, thanks.
On Wed, Apr 26, 2017 at 06:41:27PM +, Bart Van Assche wrote:
> On Wed, 2017-04-26 at 09:40 +0200, Christoph Hellwig wrote:
> > this series has some prep patches for my work to have proper, type
> > checked block errors codes. One fallout of that is that we need to
> > get rid of how dm overloa
On Tue, May 02, 2017 at 12:16:13AM -0700, Nicholas A. Bellinger wrote:
> Or, another options is use bdev_write_zeroes_sectors() to determine when
> dev_attrib->unmap_zeroes_data should be set.
Yes, that in combination with your patch to use bdev_write_zeroes_sectors
for zeroing from write same see
On Tue, May 02, 2017 at 08:33:15PM -0700, Nicholas A. Bellinger wrote:
> The larger target/iblock conversion patch looks like post v4.12 material
> at this point, so to avoid breakage wrt to existing LBPRZ behavior, I'll
> plan to push the following patch post -rc1.
I don't think this is safe. If
On Mon, May 08, 2017 at 11:46:14PM -0700, Nicholas A. Bellinger wrote:
> That said, simply propagating up q->limits.max_write_zeroes_sectors as
> dev_attrib->unmap_zeroes_data following existing code still looks like
> the right thing to do.
It is not. Martin has decoupled write same/zeroes suppo
On Wed, May 10, 2017 at 09:50:35PM -0700, Nicholas A. Bellinger wrote:
> 1) Expose a block_device or request_queue bit to signal 'real LBPRZ'
> support up to IBLOCK, in order to maintain SCSI target feature
> compatibility.
No way. If you want to zero use REQ_OP_WRITE_ZEROES..
On Thu, May 18, 2017 at 02:25:52PM +, Bart Van Assche wrote:
> On Thu, 2017-05-18 at 15:18 +0200, Christoph Hellwig wrote:
> > We've cleaned up our headers sufficiently that we don't need this split
> > anymore.
>
> Hello Christoph,
>
> Request-based drivers need the structure definitions fro
On Sun, May 21, 2017 at 02:33:05PM +, Bart Van Assche wrote:
> Thanks for the review comments. The cover letter should have made it to at
> least the linux-scsi mailing list since it shows up in at least one archive of
> that mailing list: https://www.spinics.net/lists/linux-scsi/msg108940.html
On Wed, May 24, 2017 at 04:04:40PM +, Bart Van Assche wrote:
> Are you sure that that code is not necessary? From osd_initiator.c:
>
> static void _put_request(struct request *rq)
> {
> /*
>* If osd_finalize_request() was called but the request was not
>* executed through
On Thu, May 25, 2017 at 08:19:47PM +, Bart Van Assche wrote:
> On Thu, 2017-05-25 at 14:48 -0400, J . Bruce Fields wrote:
> > On Thu, May 25, 2017 at 11:43:14AM -0700, Bart Van Assche wrote:
> > > Since using scsi_req() is only allowed against request queues for
> > > which struct scsi_request
On Fri, May 26, 2017 at 03:50:42PM +, Bart Van Assche wrote:
> On Fri, 2017-05-26 at 08:08 +0200, Christoph Hellwig wrote:
> > On Thu, May 25, 2017 at 11:43:13AM -0700, Bart Van Assche wrote:
> > > Since the cdrom driver only supports request queues for which
> > > struct scsi_request is the fi
On Fri, May 26, 2017 at 11:56:30PM +, Bart Van Assche wrote:
> I have tried to move that call into blk_mq_alloc_request() but that
> resulted in a kernel oops during boot due to scsi_add_cmd_to_list()
> dereferencing scsi_cmnd.device and due to that pointer being invalid.
> I think that pointer
On Tue, Dec 18, 2018 at 10:26:46AM -0700, Keith Busch wrote:
> No need for a space after the %s. __print_disk_name already appends a
> space if there's a disk name, and we don't want the extra space if there
> isn't one. Also, every other nvme trace has a ',' after each entry. Not
> a big deal, jus
On Tue, Dec 18, 2018 at 05:19:00PM -0800, peng yu wrote:
> I think this change is nice. Will you submit this change or are you
> suggesting me to do it?
I've folded the changes in.
Honestly I think the right fix is to just kill the SCSI passthrough
in virtio. It has been replaced by virtio-scsi a long time ago, and
has been disabled by default in qemu for a long time (and I don't think
any other hypervisor ever supported it).
It should never have been added (saying that as
On Fri, Nov 17, 2017 at 04:50:39PM +, Bart Van Assche wrote:
>
> How about the following class of assignments in drivers/md/raid1.c:
>
> mbio->bi_disk = (void *)conf->mirrors[i].rdev;
>
> Should these assignments perhaps be followed by a mbio->bi_partno assignment?
No. They a
Does commit 615d22a51c04856efe62af6e1d5b450aaf5cc2c0
"block: Fix __blkdev_issue_zeroout loop" fix the issue for you?
On Wed, Aug 23, 2017 at 06:21:55PM +, Bart Van Assche wrote:
> On Wed, 2017-08-23 at 19:58 +0200, Christoph Hellwig wrote:
> > +static blk_qc_t nvme_make_request(struct request_queue *q, struct bio *bio)
> > +{
> > + struct nvme_ns_head *head = q->queuedata;
> > + struct nvme_ns *ns;
> > +
Hi Bart,
this isn't just about performance - it's about understanability of
the I/O path.
The legacy request path already has the prep_fn, which is intended
for exactly this sort of prep work, but even that led to a lot
of confusion. For blk-mq we decided to not add it but let the called
driver
On Fri, Sep 15, 2017 at 02:51:03PM +, Bart Van Assche wrote:
> On Fri, 2017-09-15 at 19:06 +0900, Damien Le Moal wrote:
> > Rearrange sd_zbc_setup() to include use_16_for_rw and use_10_for_rw
> > assignments and move the calculation of sdkp->zone_shift together
> > with the assignment of the ve
On Mon, Sep 25, 2017 at 04:17:27PM +, Bart Van Assche wrote:
> This patch series definitely is an alternative for blk-mq/scsi-mq. And as you
> know my approach can be extended easily to the legacy SCSI core by adding
> blk_queue_enter() / blk_queue_exit() calls where necessary in the legacy blo
On Mon, Oct 02, 2017 at 03:56:08PM +, Bart Van Assche wrote:
> Since queue_flag_set() and queue_flag_clear() use non-atomic primitives to
> set and clear flags it is essential to protect calls of these functions with
> the queue lock. Otherwise flag updates could get lost due to concurrent
> qu
On Tue, Oct 03, 2017 at 03:13:43PM +, Bart Van Assche wrote:
> > + switch (rq->q->rpm_status) {
> > + case RPM_SUSPENDED:
> > + return false;
> > + case RPM_ACTIVE:
> > + return rq->rq_flags & RQF_PM;
> > + default:
> > + return true;
> > + }
> > }
>
>
On Tue, Mar 13, 2018 at 04:25:40PM +, Bart Van Assche wrote:
> On Tue, 2018-03-13 at 07:49 -0700, Jens Axboe wrote:
> > On 3/13/18 2:18 AM, Christoph Hellwig wrote:
> > > bio_check_eod() should check partiton size not the whole disk if
> > > bio->bi_partno is non-zero. Does this by taking the
Hi Bart,
can you test the version below?
---
>From a68a8518158e31d66a0dc4f4e795ca3ceb83752c Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Tue, 13 Mar 2018 09:27:30 +0100
Subject: block: bio_check_eod() needs to consider partition
bio_check_eod() should check partiton size not the whole
On Tue, Apr 10, 2018 at 01:26:39PM +, Bart Van Assche wrote:
> Can you explain why you think that using cmpxchg() is safe in this context?
> The regular completion path and the timeout code can both execute e.g. the
> following code concurrently:
>
> blk_mq_change_rq_state(rq, MQ_RQ_IN_F
On Thu, Apr 12, 2018 at 11:52:11AM +, Bart Van Assche wrote:
> On Thu, 2018-04-12 at 07:34 +0200, Christoph Hellwig wrote:
> > On Wed, Apr 11, 2018 at 07:58:52PM -0600, Bart Van Assche wrote:
> > > Several block drivers call alloc_disk() followed by put_disk() if
> > > something fails before de
On Thu, Apr 12, 2018 at 06:48:12AM -0700, t...@kernel.org wrote:
> > Which sounds like a very good reason not to use a driver controller
> > lock for internals like blkcq.
> >
> > In fact splitting the lock used for synchronizing access to queue
> > fields from the driver controller lock used to s
On Tue, Apr 17, 2018 at 05:35:07PM +, Bart Van Assche wrote:
> > Hmm. I think we need to avoid clearing that data and update it using
> > RCU instead. Calling blk_queue_enter before submitting bios is
> > something that would make zone reporting very different from any
> > other block layer u
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 a sense buffer. Just curious, what's the callstack that caused
thi
On Thu, Jan 26, 2017 at 11:57:36AM -0700, Jens Axboe wrote:
> It's against my for-4.11/block, which you were running under Christoph's
> patches. Maybe he's using an older version? In any case, should be
> pretty trivial for you to hand apply. Just ensure that .flags is set to
> 0 for the common ca
On Fri, Jan 27, 2017 at 05:58:02PM +, Bart Van Assche wrote:
> Since __scsi_init_queue() modifies data in the Scsi_Host structure, have you
> considered to add the declaration for this function to ?
> If you want to keep this declaration in please add a
> direct include of that header file to
On Fri, Jan 27, 2017 at 06:58:53PM +, Bart Van Assche wrote:
> Version 3 of the patch with title "block: split scsi_request out of
> struct request" (commit 3c30af6ebe12) differs significantly from v2
> of that patch that has been posted on several mailing lists. E.g. v2
> moves __cmd[], cmd an
On Fri, Jan 27, 2017 at 09:27:53PM +, Bart Van Assche wrote:
> Have you considered to convert all block drivers to the new
> approach and to get rid of request.special? If so, do you already
> have plans to start working on this? I'm namely wondering wheter I
> should start working on this myse
On Fri, Jan 27, 2017 at 06:39:46PM +, Bart Van Assche wrote:
> Why have the scsi_release_buffers() and scsi_put_command(cmd) calls been
> moved up? I haven't found an explanation for this change in the patch
> description.
Because they reference the scsi_cmnd, which are now part of the request
On Wed, Feb 08, 2017 at 10:43:59AM -0700, Jens Axboe wrote:
> I've changed the subject line, this issue has nothing to do with the
> issue that Hannes was attempting to fix.
Nothing really useful in the thread. Dexuan, can you throw in some
prints to see which command times out?
Hi Dexuan,
I've spent some time with the logs and looking over the code and
couldn't find any smoking gun. I start to wonder if it might just
be a timing issue?
Can you try one or two things for me:
1) run with the blk-mq I/O path for scsi by either enabling it a boot /
module load time wi
Hi Dexuan,
can you try the hack below for now? I disable the TUR call from
sd_check_events, which I think your VM is hanging on. The checks
it does on the sense data look a bit fishy, but so far I've not
identified a possible root cause.
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index
Ok, thanks for testing. Can you try the patch below? It fixes a
clear problem which was partially papered over before the commit
you bisected to, although it can't explain why blk-mq still works.
>From e4a66856fa2d92c0298000de658365f31bea60cd Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Dat
On Tue, Feb 14, 2017 at 02:46:41PM +, Dexuan Cui wrote:
> > From: h...@lst.de [mailto:h...@lst.de]
> > Sent: Tuesday, February 14, 2017 22:29
> > To: Dexuan Cui
> > Subject: Re: Boot regression (was "Re: [PATCH] genhd: Do not hold event lock
> >
> I tested today's linux-next (next-20170214) + the 2 patches just now and got
> a weird result:
> sometimes the VM stills hung with a new calltrace (BUG: spinlock bad
> magic) , but sometimes the VM did boot up despite the new calltrace!
>
> Attached is the log of a "good" boot.
>
> It looks we
On Fri, Feb 24, 2017 at 01:22:42PM -0700, Jens Axboe wrote:
> Bart, I pushed a fix here:
>
> http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=61febef40bfe8ab68259d8545257686e8a0d91d1
Yeah, this looks fine to me. It was broken on blk-mq before, but
basically impossible to hit. I wond
On Mon, Mar 13, 2017 at 08:52:54PM +, Bart Van Assche wrote:
> > - if (((plug && !blk_queue_nomerges(q)) || is_sync) &&
> > - !(data.hctx->flags & BLK_MQ_F_DEFER_ISSUE)) {
> > + if (((plug && !blk_queue_nomerges(q)) || is_sync)) {
>
> A minor comment: due to this change the outer pai
On Mon, Mar 13, 2017 at 09:01:08PM +, Bart Van Assche wrote:
> > + /*
> > +* @request_count may become stale because of schedule
> > +* out, so check the list again.
> > +*/
>
> The above comment was relevant as long as there was a request_count as
On Tue, Mar 21, 2017 at 12:40:17PM +, Bart Van Assche wrote:
> On Mon, 2017-03-20 at 16:39 -0400, Christoph Hellwig wrote:
> > Changes since V1:
> > - rebase on top of the recent blk_mq_try_issue_directly changes
> > - incorporate comments from Bart
>
> Hi Christoph,
>
> It seems to me like
On Tue, Mar 28, 2017 at 08:05:09AM -0600, ax...@kernel.dk wrote:
> > Although I know this is an issue in the existing code and not something
> > introduced by you: please consider using logical_to_sectors() instead of
> > open-coding this function. Otherwise this patch looks fine to me.
>
> The do
On Tue, Mar 28, 2017 at 04:12:46PM +, Bart Van Assche wrote:
> Since REQ_OP_WRITE_ZEROES was introduced in kernel v4.10, do we need
> "Cc: stable" and "Fixes: a6f0788ec2881" tags for this patch?
No. This just works around the way scsi_setup_cmnd sets up the data
direction. Before this series
On Tue, Mar 28, 2017 at 04:48:55PM +, Bart Van Assche wrote:
> > if (sdp->no_write_same)
> > return BLKPREP_INVALID;
> > if (sdkp->ws16 || sector > 0x || nr_sectors > 0x)
>
> Users can change the provisioning mode from user space from SD_LBP_WS16 into
> SD_LBP_W
On Tue, Mar 28, 2017 at 04:50:47PM +, Bart Van Assche wrote:
> On Thu, 2017-03-23 at 10:33 -0400, Christoph Hellwig wrote:
> > This gets us support for non-discard efficient write of zeroes (e.g. NVMe)
> > and preparse for removing the discard_zeroes_data flag.
>
> Hello Christoph,
>
> "prepa
On Tue, Mar 28, 2017 at 05:00:48PM +, Bart Van Assche wrote:
>
> It seems to me like the documentation in Documentation/ABI/testing/sysfs-block
> and the above code are not in sync. I think the above code will cause reading
> from the discard_zeroes_data attribute to return an empty string (""
On Tue, Aug 20, 2019 at 02:41:35PM +1000, Dave Chinner wrote:
> > With the following debug patch. Based on that I think I'll just
> > formally submit the vmalloc switch as we're at -rc5, and then we
> > can restart the unaligned slub allocation drama..
>
> This still doesn't make sense to me, bec
On Sat, Nov 25, 2023 at 05:38:28PM +, Michael Kelley wrote:
> Hyper-V guests and the Azure cloud have a particular interest here
> because Hyper-V guests uses SCSI as the standard interface to virtual
> disks. Azure cloud disks can be throttled to a limited number of IOPS,
> so the number of i
69 matches
Mail list logo
