On Wed, Jun 05, 2019 at 08:45:58AM -0700, Bart Van Assche wrote:
> On 6/4/19 6:06 PM, Ming Lei wrote:
> > If user doesn't ask to pre-allocate by passing zero 'nents_first_chunk' to
> > sg_alloc_table_chained, we need to make sure that 'first_chunk' is cleared.
> > Otherwise, __sg_alloc_table() still may think that the 1st SGL should
> > be from the pre-allocation.
> >
> > Fixes the issue by clearing 'first_chunk' in sg_alloc_table_chained() if
> > 'nents_first_chunk' is zero.
> >
> > Cc: Christoph Hellwig <hch@xxxxxx>
> > Cc: Bart Van Assche <bvanassche@xxxxxxx>
> > Cc: Ewan D. Milne <emilne@xxxxxxxxxx>
> > Cc: Hannes Reinecke <hare@xxxxxxxx>
> > Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> > Fixes: c3288dd8c232 ("scsi: core: avoid pre-allocating big SGL for data")
>
> Shouldn't the "Fixes:" tag be left out from this patch? I don't think that
Here the 'Fixes' means it is a fix on the code(sg_alloc_table_chained) itself.
> this patch by itself fixes anything. Isn't this patch something that is
> necessary to make patch 2/3 in this series work?
Yeah, it is needed for patch 2/3.
> How about indicating that by mentioning it in the commit message?
OK.
>
> > Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > Tested-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> > ---
> > lib/sg_pool.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/sg_pool.c b/lib/sg_pool.c
> > index 47eecbe094d8..e042a1722615 100644
> > --- a/lib/sg_pool.c
> > +++ b/lib/sg_pool.c
> > @@ -122,7 +122,7 @@ int sg_alloc_table_chained(struct sg_table *table, int nents,
> > }
> > /* User supposes that the 1st SGL includes real entry */
> > - if (nents_first_chunk == 1) {
> > + if (nents_first_chunk <= 1) {
> > first_chunk = NULL;
> > nents_first_chunk = 0;
> > }
>
> How about also updating the kernel-doc header above sg_alloc_table_chained()
> such that it is made clear that @first_chunk is ignored if nents_first_chunk
> <= 1 ? Otherwise this patch looks fine to me.
OK.
Thanks,
Ming
