Re: [PATCH 5/5] scsi: st: factor out the end-of-medium completion test

[Laurence Oberman <loberman@redhat.com>]

On Fri, 2026-11-06 at 15:42 +0800, shao.mingyin@zte.com.cn wrote:
> On 11/5/26 09:21, Bart Van Assche wrote:
> > The first occurrence is in st_flush_write_buffer(). That function
> > submits a synchronous WRITE(6) command and has nothing to do with
> > write-behind. Additionally, I count five conditions instead of six.
> > Please fix the patch description.
> 
> You are right on both counts.  I will describe it as the synchronous
> WRITE(6) issued by st_flush_write_buffer() and drop the condition
> count.
> 
> > Please declare the argument const and please choose a more
> > descriptive
> > name, e.g. st_all_written_at_eom(). The "All written at EOM early
> > warning" comment from st_flush_write_buffer() could be moved above
> > this
> > function.
> 
> Will do: const struct st_cmdstatus *cmdstatp,
> st_all_written_at_eom(), and
> the comment moved above the helper.  v2 will follow.
> 
> Thanks,
> Shao

Hi,

Thanks for the patch.

The refactor looks correct, but I don't think it buys enough to justify
touching this code. The EOM handling in st is mature and subtle around
buffered writes and residuals, and having the condition inline at each
call site, with its comment, makes it clear what "success at EOM" means
in that context. A helper named st_eom_ok() hides that.

Unless this is a prerequisite for a functional fix, I'd prefer to leave
it as is.

NAK for now.

Regards,
Laurence