On Wed, Nov 29, 2017 at 04:18:30PM +, Bart Van Assche wrote:
> As the above patch description shows it can happen that the SCSI core calls
> get_device() after the device reference count has reached zero and before
> the memory for struct device is freed. Although the above patch looks fine
> t
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 Wed, Aug 30, 2017 at 09:13:00PM +, Bart Van Assche wrote:
> On Wed, 2017-08-30 at 10:29 +0200, Christoph Hellwig wrote:
> > Looks good except for the subject, which will need a little update:
>
> Hello Christoph,
>
> Thanks for the review. Sorry but I don't see what's wrong with the subjec
On Fri, Aug 25, 2017 at 03:38:48PM +, Bart Van Assche wrote:
> On Fri, 2017-08-25 at 17:35 +0200, Christoph Hellwig wrote:
> > I looked a bit more at the history of this, and it seems like the only
> > issue with commit 17d5363b83f8 here is using the blk_status_t type for the
> > ret variable.
This looks good to me:
Reviewed-by: Christoph Hellwig
On Thu, Aug 24, 2017 at 04:27:07PM +, Bart Van Assche wrote:
>
> The purpose of that check is to avoid that dev_loss_tmo * HZ can overflow.
> That check is only needed on 32-bit systems since only on these systems
> sizeof(long) == sizeof(int). How about changing the type of the dev_loss_tmo
>
On Fri, Aug 25, 2017 at 05:58:16AM +, Seymour, Shane M wrote:
> > My understanding of the SCSI VPD code is as follows:
> > * rcu_read_lock() / rcu_read_unlock() is used to prevent that another thread
> > updates a VPD buffer while it is being read.
>
> My understanding is that it doesn't do
On Thu, Aug 24, 2017 at 04:57:03PM +, Bart Van Assche wrote:
> Is something like the patch below perhaps what you had in mind?
Yes. Except that I really don't like the sq naming - blk-mq can
also be used for single queues, so it's a rather confusing name
Something like legacy or old seems to
On Thu, Aug 24, 2017 at 04:14:22PM +, Bart Van Assche wrote:
> Commit ca18d6f769d2 ("block: Make most scsi_req_init() calls implicit")
> introduced the scsi_initialize_rq() function in such a way that it is not only
> called for pass-through requests but also for FS requests. Because that commi
On Mon, Aug 21, 2017 at 05:14:00PM -0500, Brian King wrote:
> Save / restore the retry counter in scsi_cmd in scsi_init_command.
> This allows us to go back through scsi_init_command for retries
> and not forget we are doing a retry.
So where will we initialize it to zero now?
On Mon, Aug 21, 2017 at 05:13:20PM -0500, Brian King wrote:
> Move the initialization of scsi_cmd->jiffies_at_alloc to allocation
> time rather than prep time. Also ensure that jiffies_at_alloc
> is preserved when we go through prep. This lets us send retries
> through prep again and not break the
On Thu, Aug 17, 2017 at 03:01:51PM +, Bart Van Assche wrote:
> Another possible approach is to fix the ipr driver and to resubmit the
> "scsi-mq: Always unprepare before requeuing a request" and "scsi: sd_zbc:
> Write unlock zone from sd_uninit_cmnd()" patches.
We should absolutely take that b
On Fri, Jul 14, 2017 at 05:56:41PM +0800, Jonathan Cameron wrote:
> Just wondering if you had any thoughts on how we can proceed with tracking
> down this regression?
>
> I'm not familiar enough with the multiqueue code to really know where
> to start.
>
> If there are any other tests that would
On Mon, Jun 05, 2017 at 06:36:19PM +, Bart Van Assche wrote:
> > The mpt3sas driver is the only driver that calls
> > scsi_internal_device_block()
> > and scsi_internal_device_unblock() from atomic context. Since it's not an
> > option
> > to protect the SCSI device state changes with a spinl
On Mon, May 22, 2017 at 02:56:18PM +0200, Hannes Reinecke wrote:
> > No, we don't. The driver simply sets a tag aside and doesn't expose
> > it to the block layer. Similar to what smartpqi already does for LUN
> > resets and AENs, mpt3sas does for the ioctl tags and NVMe does for AERs.
> > Person
On Mon, May 22, 2017 at 10:46:10AM +0200, Hannes Reinecke wrote:
> > That seems to be overkill to me for the few drivers. And I suspect
> > most of them would be better off now even using blk-mq private tags
> > (which we'd have to implement for the legacy path first or just
> > kill it off) but j
On Mon, May 22, 2017 at 08:06:46AM +0200, Hannes Reinecke wrote:
> Problem is that quite some LLDDs require a hardware tag for sending
> TMFs, and currently the re-use the tag from the passed in scmd for that.
> So first we need to move those to a sane interface, and having them
> requesting a new
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 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 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 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 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 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 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 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: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: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 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 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 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 Fri, Apr 07, 2017 at 07:59:08PM +, Bart Van Assche wrote:
> On Wed, 2017-04-05 at 07:41 -0400, Martin K. Petersen wrote:
> > +static ssize_t
> > +zeroing_mode_store(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct scsi
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, 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 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 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, 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 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: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 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 Thu, Feb 16, 2017 at 02:30:34PM +, Bart Van Assche wrote:
> > sdev = q->queuedata;
> > - if (!sdev || !get_device(&sdev->sdev_gendev))
> > + if (!sdev ||
> > + !scsi_is_sdev_device(&sdev->sdev_gendev) ||
> > + !get_device(&sdev->sdev_gendev))
> > sdev = NULL;
On Tue, Feb 14, 2017 at 02:21:37PM +0900, Damien Le Moal wrote:
> > I think we want to keep the knowledge of which requests have a request size
> > that should be a multiple of the logical block size in the block layer core
> > or in the SCSI core but not in the mpt3sas driver. But I'm not sure wha
On Mon, Jan 30, 2017 at 04:46:00PM +, Bart Van Assche wrote:
> Patches 1, 2 and 4 of this series are a real improvement in my opinion.
> However, I'm not sure whether patch 3 is the best way to avoid that drivers
> have to re-override the transport template timeout handler. Have you
> considere
Hi Bart,
thanks for the analysis. I think the main issue here is that we shouldn't
try to assign affinity masks if there are not affinity vectors left to
assign.
Does the patch below fix the issue for you?
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 50c5003..96c0713 100644
--- a/dr
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: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 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 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 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 Wed, Jan 25, 2017 at 03:47:20PM +, Bart Van Assche wrote:
> =
> BUG kmalloc-16 (Not tainted): Redzone overwritten
> -
>
> Disabling lock de
On Wed, Jan 25, 2017 at 10:44:41PM +, Bart Van Assche wrote:
> On Wed, 2017-01-25 at 11:37 +0100, Christoph Hellwig wrote:
> > > Fixes: f80de881d8df ("sd: remove __data_len hack for WRITE SAME")
> >
> > I think the better fix is to simply revert this commit.
>
> Hello Christoph,
>
> Do you m
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 Mon, Feb 02, 2015 at 03:30:27PM +, James Bottomley wrote:
> Cc added for linux-scsi, since this is the origin of the problem. How
> important is bisectability in this? It won't affect any non-embedded
> user, since most don't build with UFS, so I can go either way on folding
> or just appl
On Tue, Jan 13, 2015 at 06:56:17PM +, James Bottomley wrote:
> I really hate using an on-stack buffer here ... we're pretty deep in the
> call chain already.
>
> Since it's just required for printing a "command: " prefix, why not just
> use scsi_print_command()?
Both the ch and sr callers are
On Wed, Dec 03, 2014 at 03:42:19PM +, James Bottomley wrote:
> That's the problem: The driver as-is works in all of the use cases, so
> no-one has any motivation to fix it. Plus the complexity is pretty
> daunting. It's not even the worst driver we have (that prize goes to
> atp870u which sti
54 matches
Mail list logo
