Re: [PATCH 1/2] scsi: sd: Check physical sector alignment of sequential zone writes

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

On Mar 04, 2023 / 07:21, Bart Van Assche wrote:
> On 3/3/23 19:03, Damien Le Moal wrote:
> > On 3/4/23 03:03, Bart Van Assche wrote:
> > > On 3/2/23 17:44, Shin'ichiro Kawasaki wrote:
> > > > +	if (sdkp->device->type == TYPE_ZBC && blk_rq_zone_is_seq(rq) &&
> > > > +	    (req_op(rq) == REQ_OP_WRITE || req_op(rq) == REQ_OP_ZONE_APPEND) &&
> > > > +	    (!IS_ALIGNED(blk_rq_pos(rq), pb_sectors) ||
> > > > +	     !IS_ALIGNED(blk_rq_sectors(rq), pb_sectors))) {
> > > > +		scmd_printk(KERN_ERR, cmd,
> > > > +			    "Sequential write request not aligned to the physical block size\n");
> > > > +		goto fail;
> > > > +	}
> > > 
> > > I vote -1 for this patch because my opinion is that we should not
> > > duplicate checks that must be performed by the storage controller anyway
> > > inside the sd driver.
> > 
> > Sure, the drive will fail this request, so the end result is the same. But what
> > is the point of issuing such unaligned request that we know will fail ? The
> > error message also make it easier to debug as it clarifies that this is not a
> > write pointer violation. So while this change is not critical, it does have
> > merits in my opinion.
> 
> I think that there are other ways to debug software that triggers an
> unaligned write, e.g. ftrace.

I see, then let me drop this patch. I will repost the second patch only for
reviews.

-- 
Shin'ichiro Kawasaki