Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-16 Thread Guilherme G. Piccoli
On 15/02/2017 05:06, Ram Pai wrote: > On Wed, Feb 15, 2017 at 03:48:52PM +0900, Damien Le Moal wrote: >> Christoph, >> >> On 2/15/17 15:34, Christoph Hellwig wrote: >>> this looks reasonable, but we should ask Guilherme and Ram to confirm >>> it fixes their originally reported issue. I've added

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Damien Le Moal
Bart, On 2/16/17 12:28, Bart Van Assche wrote: > On Thu, 2017-02-16 at 11:52 +0900, Damien Le Moal wrote: >> Thanks for the pointers. I checked libiscsi tests. And from what is done >> there, it seems to me that it is basically impossible to distinguished >> between a buggy hardware response and

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Damien Le Moal
Martin, On 2/16/17 12:36, Martin K. Petersen wrote: > Damien, > > Damien> So the conclusion may be that we need to drop everything ? The > Damien> mpt3sas patch breaks ZBC now, so that must be removed too. > > Nah. > > But it's important that we restrict the rounding to TYPE_FS requests >

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Martin K. Petersen
> "Damien" == Damien Le Moal writes: Damien, Damien> So the conclusion may be that we need to drop everything ? The Damien> mpt3sas patch breaks ZBC now, so that must be removed too. Nah. But it's important that we restrict the rounding to TYPE_FS requests (i.e.

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Bart Van Assche
On Thu, 2017-02-16 at 11:52 +0900, Damien Le Moal wrote: > Thanks for the pointers. I checked libiscsi tests. And from what is done > there, it seems to me that it is basically impossible to distinguished > between a buggy hardware response and an in-purpose (or buggy) not > aligned data-out

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Damien Le Moal
Bart, On 2/16/17 11:52, Damien Le Moal wrote: > Bart, > > On 2/16/17 10:10, Bart Van Assche wrote: >> On Thu, 2017-02-16 at 09:54 +0900, Damien Le Moal wrote: >>> On 2/16/17 01:42, Bart Van Assche wrote: An additional concern: what if the size of the Data-Out buffer is not a multiple

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Damien Le Moal
Bart, On 2/16/17 10:10, Bart Van Assche wrote: > On Thu, 2017-02-16 at 09:54 +0900, Damien Le Moal wrote: >> On 2/16/17 01:42, Bart Van Assche wrote: >>> An additional concern: what if the size of the Data-Out buffer is not a >>> multiple of the logical block size? Shouldn't we round down

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Bart Van Assche
On Thu, 2017-02-16 at 09:54 +0900, Damien Le Moal wrote: > On 2/16/17 01:42, Bart Van Assche wrote: > > An additional concern: what if the size of the Data-Out buffer is not a > > multiple of the logical block size? Shouldn't we round down (good_bytes - > > resid) instead of rounding up resid? >

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Damien Le Moal
Bart, On 2/16/17 01:42, Bart Van Assche wrote: > On Wed, 2017-02-15 at 11:12 +0900, Damien Le Moal wrote: >> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c >> index 1f5d92a..d05a328 100644 >> --- a/drivers/scsi/sd.c >> +++ b/drivers/scsi/sd.c >> @@ -1790,6 +1790,8 @@ static int sd_done(struct

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Damien Le Moal
Bart, On 2/16/17 00:10, Bart Van Assche wrote: > On Wed, 2017-02-15 at 11:12 +0900, Damien Le Moal wrote: >> +resid = round_up(resid, sector_size); >> +if (resid < good_bytes) >> +good_bytes -= resid; >> +else

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Bart Van Assche
On Wed, 2017-02-15 at 11:12 +0900, Damien Le Moal wrote: > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 1f5d92a..d05a328 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -1790,6 +1790,8 @@ static int sd_done(struct scsi_cmnd *SCpnt) > { > int result =

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-15 Thread Bart Van Assche
On Wed, 2017-02-15 at 11:12 +0900, Damien Le Moal wrote: > + resid = round_up(resid, sector_size); > + if (resid < good_bytes) > + good_bytes -= resid; > + else > + good_bytes = 0; >

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-14 Thread Ram Pai
On Wed, Feb 15, 2017 at 03:48:52PM +0900, Damien Le Moal wrote: > Christoph, > > On 2/15/17 15:34, Christoph Hellwig wrote: > > this looks reasonable, but we should ask Guilherme and Ram to confirm > > it fixes their originally reported issue. I've added them to Cc. > > Thank you. > >

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-14 Thread Damien Le Moal
Christoph, On 2/15/17 15:34, Christoph Hellwig wrote: > this looks reasonable, but we should ask Guilherme and Ram to confirm > it fixes their originally reported issue. I've added them to Cc. Thank you. Guilherme, Ram, Please test ! The original fix breaks the zoned block device support that

Re: [PATCH v4] sd: Check for unaligned partial completion

2017-02-14 Thread Christoph Hellwig
Hi Damien, this looks reasonable, but we should ask Guilherme and Ram to confirm it fixes their originally reported issue. I've added them to Cc. On Wed, Feb 15, 2017 at 11:12:30AM +0900, Damien Le Moal wrote: > Commit "mpt3sas: Force request partial completion alignment" was not > considering

[PATCH v4] sd: Check for unaligned partial completion

2017-02-14 Thread Damien Le Moal
Commit "mpt3sas: Force request partial completion alignment" was not considering the case of REQ_TYPE_FS commands not operating on sector size units (e.g. REQ_OP_ZONE_REPORT and its 64B aligned partial replies). This could result is incorrectly retrying (forever) those commands. Move the partial