| Message ID | 20220825213900.864587-1-bvanassche@acm.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | RDMA/srp: Handle dev_set_name() failure | expand |
On Thu, Aug 25, 2022 at 02:38:56PM -0700, Bart Van Assche wrote: > Hi Jason, > > 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. Thanks > > Thanks, > > Bart. > > Bart Van Assche (4): > RDMA/srp: Rework the srp_add_port() error path > RDMA/srp: Remove the srp_host.released completion > RDMA/srp: Handle dev_set_name() failure > RDMA/srp: Use the attribute group mechanism for sysfs attributes > > drivers/infiniband/ulp/srp/ib_srp.c | 50 +++++++++++++++-------------- > drivers/infiniband/ulp/srp/ib_srp.h | 1 - > 2 files changed, 26 insertions(+), 25 deletions(-) >
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. 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. Thanks, Bart.
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.
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. > > 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. Generally I always want to see the Fixes tag because it aides everyone in the ecosystem to know when they should consider the patch for backporting with a simple uniform algorithm. So marking the first commit as the thing being fixed is expected. Jason