Re: [PATCH 0/4] RDMA/srp: Handle dev_set_name() failure

[Leon Romanovsky <leon@kernel.org>]

On Sun, Aug 28, 2022 at 12:50:28PM -0700, Bart Van Assche wrote:
> On 8/28/22 03:04, Leon Romanovsky wrote:
> > On Thu, Aug 25, 2022 at 02:38:56PM -0700, Bart Van Assche wrote:
> > > This patch series includes one patch that handles dev_set_name() failure and
> > > three refactoring patches. Please consider these patches for the next merge
> > > window.
> > 
> > You confuse me. "next merge window" means that patches are targeted to
> > -next, but you added stable@... tag and didn't add any Fixes lines.
> > 
> > I applied everything to rdma-next and removed stable@ tag.
> 
> Hi Leon,
> 
> Although it's not a big deal for this patch series, please do not modify patches
> without agreement from the patch author.

I didn't promote the series from my WIP branch to for-next yet and can drop
them, if you want.

> 
> As far as I know adding a Fixes: tag if a Cc: stable tag is present is not required
> by any document in the Documentation/ directory?
> 
> I had not added a Fixes: tag because the issue fixed by patch 3/3 was introduced
> by the commit that added the ib_srp driver to the kernel tree. So it would be fine
> to backport the first three patches of this series to all older kernel versions to
> which the patches can be backported.

You wanted third patch in stable@, but didn't add tag to it or any
indication that it must be there. Instead of it, you added stable@
to some cleanup that would be backported anyway if third patch would
be stable material.

Let's me cite Documentation/process/stable-kernel-rules.rst with items
that make this series is not suitable for stable:
...
   12  - It must fix a real bug that bothers people (not a, "This could be a
   13    problem..." type thing).
   14  - It must fix a problem that causes a build error (but not for things
   15    marked CONFIG_BROKEN), an oops, a hang, data corruption, a real
   16    security issue, or some "oh, that's not good" issue.  In short, something
   17    critical.
   18  - Serious issues as reported by a user of a distribution kernel may also
   19    be considered if they fix a notable performance or interactivity issue.
   20    As these fixes are not as obvious and have a higher risk of a subtle
   21    regression they should only be submitted by a distribution kernel
   22    maintainer and include an addendum linking to a bugzilla entry if it
   23    exists and additional information on the user-visible impact.
   24  - New device IDs and quirks are also accepted.
...
   25  - No "theoretical race condition" issues, unless an explanation of how the
   26    race can be exploited is also provided.
   29  - It must follow the 
   30    :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`
   31    rules.

Documentation/process/submitting-patches.rst:
...
  137 If your patch fixes a bug in a specific commit, e.g. you found an issue using
  138 ``git bisect``, please use the 'Fixes:' tag with the first 12 characters of
  139 the SHA-1 ID, and the one line summary. 
...

Also I hope that you looked when dev_set_name() can fail. Hint, when it
failed to allocate enough room for short string "srp-%s-%d". If it is
happened, you have much more serious problems than not-checked
dev_set_name().

Why is it so urgent to be part of stable? Can you present me the case
where user had OOM during dev_set_name at the beginning of srp initialization
routine and passed device_register() later?

Thanks

> 
> Thanks,
> 
> Bart.