[PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Gal Pressman]

The new attribute indicates that the kernel copies DMA pages on fork,
hence libibverbs' fork support through madvise and MADV_DONTFORK is not
needed.

The introduced attribute is always reported as supported since the
kernel has the patch that added the copy-on-fork behavior. This allows
the userspace library to identify older vs newer kernel versions.
Extra care should be taken when backporting this patch as it relies on
the fact that the copy-on-fork patch is merged, hence no check for
support is added.

Copy-on-fork attribute is read-only, trying to change it through the set
sys command will result in an error.

Signed-off-by: Gal Pressman <galpress@amazon.com>
---
PR was sent:
https://github.com/linux-rdma/rdma-core/pull/975
---
 drivers/infiniband/core/nldev.c  | 19 ++++++++++++++-----
 include/uapi/rdma/rdma_netlink.h |  2 ++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index b8dc002a2478..87c68301c25b 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -146,6 +146,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_UVERBS_DRIVER_ID]	= { .type = NLA_U32 },
 	[RDMA_NLDEV_NET_NS_FD]			= { .type = NLA_U32 },
 	[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]	= { .type = NLA_U8 },
+	[RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK]	= { .type = NLA_U8 },
 };
 
 static int put_driver_name_print_type(struct sk_buff *msg, const char *name,
@@ -1693,12 +1694,19 @@ static int nldev_sys_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_NETNS_MODE,
 			 (u8)ib_devices_shared_netns);
-	if (err) {
-		nlmsg_free(msg);
-		return err;
-	}
+	if (err)
+		goto err_nlmsg_free;
+
+	err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK, 1);
+	if (err)
+		goto err_nlmsg_free;
+
 	nlmsg_end(msg, nlh);
 	return rdma_nl_unicast(sock_net(skb->sk), msg, NETLINK_CB(skb).portid);
+
+err_nlmsg_free:
+	nlmsg_free(msg);
+	return err;
 }
 
 static int nldev_set_sys_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
@@ -1710,7 +1718,8 @@ static int nldev_set_sys_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 
 	err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
 			  nldev_policy, extack);
-	if (err || !tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE])
+	if (err || !tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE] ||
+	    tb[RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK])
 		return -EINVAL;
 
 	enable = nla_get_u8(tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]);
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index d2f5b8396243..342c9db5b3c1 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -533,6 +533,8 @@ enum rdma_nldev_attr {
 
 	RDMA_NLDEV_ATTR_RES_RAW,	/* binary */
 
+	RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,	/* u8 */
+
 	/*
 	 * Always the end
 	 */

base-commit: adb76a520d068a54ee5ca82e756cf8e5a47363a4
-- 
2.31.1

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Leon Romanovsky]

On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
> The new attribute indicates that the kernel copies DMA pages on fork,
> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
> needed.
> 
> The introduced attribute is always reported as supported since the
> kernel has the patch that added the copy-on-fork behavior. This allows
> the userspace library to identify older vs newer kernel versions.
> Extra care should be taken when backporting this patch as it relies on
> the fact that the copy-on-fork patch is merged, hence no check for
> support is added.

Please be more specific, add SHA-1 of that patch and wrote the same
comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
1);" line.

Thanks

> 
> Copy-on-fork attribute is read-only, trying to change it through the set
> sys command will result in an error.
> 
> Signed-off-by: Gal Pressman <galpress@amazon.com>
> ---
> PR was sent:
> https://github.com/linux-rdma/rdma-core/pull/975
> ---
>  drivers/infiniband/core/nldev.c  | 19 ++++++++++++++-----
>  include/uapi/rdma/rdma_netlink.h |  2 ++
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> index b8dc002a2478..87c68301c25b 100644
> --- a/drivers/infiniband/core/nldev.c
> +++ b/drivers/infiniband/core/nldev.c
> @@ -146,6 +146,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
>  	[RDMA_NLDEV_ATTR_UVERBS_DRIVER_ID]	= { .type = NLA_U32 },
>  	[RDMA_NLDEV_NET_NS_FD]			= { .type = NLA_U32 },
>  	[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]	= { .type = NLA_U8 },
> +	[RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK]	= { .type = NLA_U8 },
>  };
>  
>  static int put_driver_name_print_type(struct sk_buff *msg, const char *name,
> @@ -1693,12 +1694,19 @@ static int nldev_sys_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
>  
>  	err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_NETNS_MODE,
>  			 (u8)ib_devices_shared_netns);
> -	if (err) {
> -		nlmsg_free(msg);
> -		return err;
> -	}
> +	if (err)
> +		goto err_nlmsg_free;
> +
> +	err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK, 1);
> +	if (err)
> +		goto err_nlmsg_free;

Is it important to have an ability to fail here? Can we simply ignore
failure?

> +
>  	nlmsg_end(msg, nlh);
>  	return rdma_nl_unicast(sock_net(skb->sk), msg, NETLINK_CB(skb).portid);
> +
> +err_nlmsg_free:
> +	nlmsg_free(msg);
> +	return err;
>  }
>  
>  static int nldev_set_sys_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
> @@ -1710,7 +1718,8 @@ static int nldev_set_sys_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
>  
>  	err = nlmsg_parse(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
>  			  nldev_policy, extack);
> -	if (err || !tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE])
> +	if (err || !tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE] ||
> +	    tb[RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK])
>  		return -EINVAL;
>  
>  	enable = nla_get_u8(tb[RDMA_NLDEV_SYS_ATTR_NETNS_MODE]);
> diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
> index d2f5b8396243..342c9db5b3c1 100644
> --- a/include/uapi/rdma/rdma_netlink.h
> +++ b/include/uapi/rdma/rdma_netlink.h
> @@ -533,6 +533,8 @@ enum rdma_nldev_attr {
>  
>  	RDMA_NLDEV_ATTR_RES_RAW,	/* binary */
>  
> +	RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,	/* u8 */
> +
>  	/*
>  	 * Always the end
>  	 */
> 
> base-commit: adb76a520d068a54ee5ca82e756cf8e5a47363a4
> -- 
> 2.31.1
> 

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Leon Romanovsky]

On Mon, Apr 05, 2021 at 02:57:05PM +0300, Leon Romanovsky wrote:
> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
> > The new attribute indicates that the kernel copies DMA pages on fork,
> > hence libibverbs' fork support through madvise and MADV_DONTFORK is not
> > needed.
> > 
> > The introduced attribute is always reported as supported since the
> > kernel has the patch that added the copy-on-fork behavior. This allows
> > the userspace library to identify older vs newer kernel versions.
> > Extra care should be taken when backporting this patch as it relies on
> > the fact that the copy-on-fork patch is merged, hence no check for
> > support is added.
> 
> Please be more specific, add SHA-1 of that patch and wrote the same
> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
> 1);" line.

And rdmatool should be updated too.

Thanks

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Gal Pressman]

On 05/04/2021 14:57, Leon Romanovsky wrote:
> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
>> The new attribute indicates that the kernel copies DMA pages on fork,
>> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
>> needed.
>>
>> The introduced attribute is always reported as supported since the
>> kernel has the patch that added the copy-on-fork behavior. This allows
>> the userspace library to identify older vs newer kernel versions.
>> Extra care should be taken when backporting this patch as it relies on
>> the fact that the copy-on-fork patch is merged, hence no check for
>> support is added.
> 
> Please be more specific, add SHA-1 of that patch and wrote the same
> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
> 1);" line.
> 
> Thanks

Should I put the original commit here? There were quite a lot of bug fixes and
followups that are required.

>>
>> Copy-on-fork attribute is read-only, trying to change it through the set
>> sys command will result in an error.
>>
>> Signed-off-by: Gal Pressman <galpress@amazon.com>
>> ---
>> PR was sent:
>> https://github.com/linux-rdma/rdma-core/pull/975
>> ---
>>  drivers/infiniband/core/nldev.c  | 19 ++++++++++++++-----
>>  include/uapi/rdma/rdma_netlink.h |  2 ++
>>  2 files changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
>> index b8dc002a2478..87c68301c25b 100644
>> --- a/drivers/infiniband/core/nldev.c
>> +++ b/drivers/infiniband/core/nldev.c
>> @@ -146,6 +146,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
>>       [RDMA_NLDEV_ATTR_UVERBS_DRIVER_ID]      = { .type = NLA_U32 },
>>       [RDMA_NLDEV_NET_NS_FD]                  = { .type = NLA_U32 },
>>       [RDMA_NLDEV_SYS_ATTR_NETNS_MODE]        = { .type = NLA_U8 },
>> +     [RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK]      = { .type = NLA_U8 },
>>  };
>>
>>  static int put_driver_name_print_type(struct sk_buff *msg, const char *name,
>> @@ -1693,12 +1694,19 @@ static int nldev_sys_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
>>
>>       err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_NETNS_MODE,
>>                        (u8)ib_devices_shared_netns);
>> -     if (err) {
>> -             nlmsg_free(msg);
>> -             return err;
>> -     }
>> +     if (err)
>> +             goto err_nlmsg_free;
>> +
>> +     err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK, 1);
>> +     if (err)
>> +             goto err_nlmsg_free;
> 
> Is it important to have an ability to fail here? Can we simply ignore
> failure?

Should be fine.

Thanks

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Gal Pressman]

On 05/04/2021 14:58, Leon Romanovsky wrote:
> On Mon, Apr 05, 2021 at 02:57:05PM +0300, Leon Romanovsky wrote:
>> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
>>> The new attribute indicates that the kernel copies DMA pages on fork,
>>> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
>>> needed.
>>>
>>> The introduced attribute is always reported as supported since the
>>> kernel has the patch that added the copy-on-fork behavior. This allows
>>> the userspace library to identify older vs newer kernel versions.
>>> Extra care should be taken when backporting this patch as it relies on
>>> the fact that the copy-on-fork patch is merged, hence no check for
>>> support is added.
>>
>> Please be more specific, add SHA-1 of that patch and wrote the same
>> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
>> 1);" line.
> 
> And rdmatool should be updated too.

Sure.

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Leon Romanovsky]

On Mon, Apr 05, 2021 at 03:15:18PM +0300, Gal Pressman wrote:
> On 05/04/2021 14:57, Leon Romanovsky wrote:
> > On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
> >> The new attribute indicates that the kernel copies DMA pages on fork,
> >> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
> >> needed.
> >>
> >> The introduced attribute is always reported as supported since the
> >> kernel has the patch that added the copy-on-fork behavior. This allows
> >> the userspace library to identify older vs newer kernel versions.
> >> Extra care should be taken when backporting this patch as it relies on
> >> the fact that the copy-on-fork patch is merged, hence no check for
> >> support is added.
> > 
> > Please be more specific, add SHA-1 of that patch and wrote the same
> > comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
> > 1);" line.
> > 
> > Thanks
> 
> Should I put the original commit here? There were quite a lot of bug fixes and
> followups that are required.

IMHO, the last commit SHA will be enough, the one that has working
functionality from your POV.

Thanks

> 
> >>
> >> Copy-on-fork attribute is read-only, trying to change it through the set
> >> sys command will result in an error.
> >>
> >> Signed-off-by: Gal Pressman <galpress@amazon.com>
> >> ---
> >> PR was sent:
> >> https://github.com/linux-rdma/rdma-core/pull/975
> >> ---
> >>  drivers/infiniband/core/nldev.c  | 19 ++++++++++++++-----
> >>  include/uapi/rdma/rdma_netlink.h |  2 ++
> >>  2 files changed, 16 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> >> index b8dc002a2478..87c68301c25b 100644
> >> --- a/drivers/infiniband/core/nldev.c
> >> +++ b/drivers/infiniband/core/nldev.c
> >> @@ -146,6 +146,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
> >>       [RDMA_NLDEV_ATTR_UVERBS_DRIVER_ID]      = { .type = NLA_U32 },
> >>       [RDMA_NLDEV_NET_NS_FD]                  = { .type = NLA_U32 },
> >>       [RDMA_NLDEV_SYS_ATTR_NETNS_MODE]        = { .type = NLA_U8 },
> >> +     [RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK]      = { .type = NLA_U8 },
> >>  };
> >>
> >>  static int put_driver_name_print_type(struct sk_buff *msg, const char *name,
> >> @@ -1693,12 +1694,19 @@ static int nldev_sys_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
> >>
> >>       err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_NETNS_MODE,
> >>                        (u8)ib_devices_shared_netns);
> >> -     if (err) {
> >> -             nlmsg_free(msg);
> >> -             return err;
> >> -     }
> >> +     if (err)
> >> +             goto err_nlmsg_free;
> >> +
> >> +     err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK, 1);
> >> +     if (err)
> >> +             goto err_nlmsg_free;
> > 
> > Is it important to have an ability to fail here? Can we simply ignore
> > failure?
> 
> Should be fine.
> 
> Thanks

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Gal Pressman]

On 05/04/2021 15:39, Leon Romanovsky wrote:
> On Mon, Apr 05, 2021 at 03:15:18PM +0300, Gal Pressman wrote:
>> On 05/04/2021 14:57, Leon Romanovsky wrote:
>>> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
>>>> The new attribute indicates that the kernel copies DMA pages on fork,
>>>> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
>>>> needed.
>>>>
>>>> The introduced attribute is always reported as supported since the
>>>> kernel has the patch that added the copy-on-fork behavior. This allows
>>>> the userspace library to identify older vs newer kernel versions.
>>>> Extra care should be taken when backporting this patch as it relies on
>>>> the fact that the copy-on-fork patch is merged, hence no check for
>>>> support is added.
>>>
>>> Please be more specific, add SHA-1 of that patch and wrote the same
>>> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
>>> 1);" line.
>>>
>>> Thanks
>>
>> Should I put the original commit here? There were quite a lot of bug fixes and
>> followups that are required.
> 
> IMHO, the last commit SHA will be enough, the one that has working
> functionality from your POV.

OK, so that would be:
4eae4efa2c29 ("hugetlb: do early cow when page pinned on src mm")

Which I now realize for-next isn't rebased on top of it yet, so these patches
should be applied after rebasing to v5.12-rc5.

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Jason Gunthorpe]

On Mon, Apr 05, 2021 at 04:09:39PM +0300, Gal Pressman wrote:
> On 05/04/2021 15:39, Leon Romanovsky wrote:
> > On Mon, Apr 05, 2021 at 03:15:18PM +0300, Gal Pressman wrote:
> >> On 05/04/2021 14:57, Leon Romanovsky wrote:
> >>> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
> >>>> The new attribute indicates that the kernel copies DMA pages on fork,
> >>>> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
> >>>> needed.
> >>>>
> >>>> The introduced attribute is always reported as supported since the
> >>>> kernel has the patch that added the copy-on-fork behavior. This allows
> >>>> the userspace library to identify older vs newer kernel versions.
> >>>> Extra care should be taken when backporting this patch as it relies on
> >>>> the fact that the copy-on-fork patch is merged, hence no check for
> >>>> support is added.
> >>>
> >>> Please be more specific, add SHA-1 of that patch and wrote the same
> >>> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
> >>> 1);" line.
> >>>
> >>> Thanks
> >>
> >> Should I put the original commit here? There were quite a lot of bug fixes and
> >> followups that are required.
> > 
> > IMHO, the last commit SHA will be enough, the one that has working
> > functionality from your POV.
> 
> OK, so that would be:
> 4eae4efa2c29 ("hugetlb: do early cow when page pinned on src mm")

No, lets put them all

And I'd mark them with a Fixes: and a huge comment saying not to
backport this and that the Fixes lines exist to *prevent* tooling from
wrongly backporting to kernels that cannot support this.

And email Sasha to see if there is a magic text we can add to prevent
auto backporting

> Which I now realize for-next isn't rebased on top of it yet, so these patches
> should be applied after rebasing to v5.12-rc5.

I will sort it out

Jason

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Leon Romanovsky]

On Mon, Apr 05, 2021 at 07:15:32PM -0300, Jason Gunthorpe wrote:
> On Mon, Apr 05, 2021 at 04:09:39PM +0300, Gal Pressman wrote:
> > On 05/04/2021 15:39, Leon Romanovsky wrote:
> > > On Mon, Apr 05, 2021 at 03:15:18PM +0300, Gal Pressman wrote:
> > >> On 05/04/2021 14:57, Leon Romanovsky wrote:
> > >>> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
> > >>>> The new attribute indicates that the kernel copies DMA pages on fork,
> > >>>> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
> > >>>> needed.
> > >>>>
> > >>>> The introduced attribute is always reported as supported since the
> > >>>> kernel has the patch that added the copy-on-fork behavior. This allows
> > >>>> the userspace library to identify older vs newer kernel versions.
> > >>>> Extra care should be taken when backporting this patch as it relies on
> > >>>> the fact that the copy-on-fork patch is merged, hence no check for
> > >>>> support is added.
> > >>>
> > >>> Please be more specific, add SHA-1 of that patch and wrote the same
> > >>> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
> > >>> 1);" line.
> > >>>
> > >>> Thanks
> > >>
> > >> Should I put the original commit here? There were quite a lot of bug fixes and
> > >> followups that are required.
> > > 
> > > IMHO, the last commit SHA will be enough, the one that has working
> > > functionality from your POV.
> > 
> > OK, so that would be:
> > 4eae4efa2c29 ("hugetlb: do early cow when page pinned on src mm")
> 
> No, lets put them all

The more data the better chance that it will be missed.
It is much saner to add last commit.

> 
> And I'd mark them with a Fixes: and a huge comment saying not to
> backport this and that the Fixes lines exist to *prevent* tooling from
> wrongly backporting to kernels that cannot support this.
> 
> And email Sasha to see if there is a magic text we can add to prevent
> auto backporting

Don't add Fixes, maybe?


> 
> > Which I now realize for-next isn't rebased on top of it yet, so these patches
> > should be applied after rebasing to v5.12-rc5.
> 
> I will sort it out
> 
> Jason

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Gal Pressman]

On 06/04/2021 9:44, Leon Romanovsky wrote:
> On Mon, Apr 05, 2021 at 07:15:32PM -0300, Jason Gunthorpe wrote:
>> On Mon, Apr 05, 2021 at 04:09:39PM +0300, Gal Pressman wrote:
>>> On 05/04/2021 15:39, Leon Romanovsky wrote:
>>>> On Mon, Apr 05, 2021 at 03:15:18PM +0300, Gal Pressman wrote:
>>>>> On 05/04/2021 14:57, Leon Romanovsky wrote:
>>>>>> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
>>>>>>> The new attribute indicates that the kernel copies DMA pages on fork,
>>>>>>> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
>>>>>>> needed.
>>>>>>>
>>>>>>> The introduced attribute is always reported as supported since the
>>>>>>> kernel has the patch that added the copy-on-fork behavior. This allows
>>>>>>> the userspace library to identify older vs newer kernel versions.
>>>>>>> Extra care should be taken when backporting this patch as it relies on
>>>>>>> the fact that the copy-on-fork patch is merged, hence no check for
>>>>>>> support is added.
>>>>>>
>>>>>> Please be more specific, add SHA-1 of that patch and wrote the same
>>>>>> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
>>>>>> 1);" line.
>>>>>>
>>>>>> Thanks
>>>>>
>>>>> Should I put the original commit here? There were quite a lot of bug fixes and
>>>>> followups that are required.
>>>>
>>>> IMHO, the last commit SHA will be enough, the one that has working
>>>> functionality from your POV.
>>>
>>> OK, so that would be:
>>> 4eae4efa2c29 ("hugetlb: do early cow when page pinned on src mm")
>>
>> No, lets put them all
> 
> The more data the better chance that it will be missed.
> It is much saner to add last commit.

I can gather a list of commits, but I'm not sure I'm aware of every single
commit that's needed to make this work properly, there's a chance some will be
missed.

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Jason Gunthorpe]

On Tue, Apr 06, 2021 at 09:44:16AM +0300, Leon Romanovsky wrote:
> On Mon, Apr 05, 2021 at 07:15:32PM -0300, Jason Gunthorpe wrote:
> > On Mon, Apr 05, 2021 at 04:09:39PM +0300, Gal Pressman wrote:
> > > On 05/04/2021 15:39, Leon Romanovsky wrote:
> > > > On Mon, Apr 05, 2021 at 03:15:18PM +0300, Gal Pressman wrote:
> > > >> On 05/04/2021 14:57, Leon Romanovsky wrote:
> > > >>> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
> > > >>>> The new attribute indicates that the kernel copies DMA pages on fork,
> > > >>>> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
> > > >>>> needed.
> > > >>>>
> > > >>>> The introduced attribute is always reported as supported since the
> > > >>>> kernel has the patch that added the copy-on-fork behavior. This allows
> > > >>>> the userspace library to identify older vs newer kernel versions.
> > > >>>> Extra care should be taken when backporting this patch as it relies on
> > > >>>> the fact that the copy-on-fork patch is merged, hence no check for
> > > >>>> support is added.
> > > >>>
> > > >>> Please be more specific, add SHA-1 of that patch and wrote the same
> > > >>> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
> > > >>> 1);" line.
> > > >>>
> > > >>> Thanks
> > > >>
> > > >> Should I put the original commit here? There were quite a lot of bug fixes and
> > > >> followups that are required.
> > > > 
> > > > IMHO, the last commit SHA will be enough, the one that has working
> > > > functionality from your POV.
> > > 
> > > OK, so that would be:
> > > 4eae4efa2c29 ("hugetlb: do early cow when page pinned on src mm")
> > 
> > No, lets put them all
> 
> The more data the better chance that it will be missed.
> It is much saner to add last commit.

I don't know, there is alot of automation here we need to ensure that
none of it triggers unless all the required commits are present. I
would list all the tops of all the 'fixes' chains.

> > And email Sasha to see if there is a magic text we can add to prevent
> > auto backporting
> 
> Don't add Fixes, maybe?

Then it randomly AI matches on the commit text and we loose even the
protection of the fixes lines

Jason

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Jason Gunthorpe]

On Tue, Apr 06, 2021 at 05:31:38PM +0300, Gal Pressman wrote:
> On 06/04/2021 9:44, Leon Romanovsky wrote:
> > On Mon, Apr 05, 2021 at 07:15:32PM -0300, Jason Gunthorpe wrote:
> >> On Mon, Apr 05, 2021 at 04:09:39PM +0300, Gal Pressman wrote:
> >>> On 05/04/2021 15:39, Leon Romanovsky wrote:
> >>>> On Mon, Apr 05, 2021 at 03:15:18PM +0300, Gal Pressman wrote:
> >>>>> On 05/04/2021 14:57, Leon Romanovsky wrote:
> >>>>>> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
> >>>>>>> The new attribute indicates that the kernel copies DMA pages on fork,
> >>>>>>> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
> >>>>>>> needed.
> >>>>>>>
> >>>>>>> The introduced attribute is always reported as supported since the
> >>>>>>> kernel has the patch that added the copy-on-fork behavior. This allows
> >>>>>>> the userspace library to identify older vs newer kernel versions.
> >>>>>>> Extra care should be taken when backporting this patch as it relies on
> >>>>>>> the fact that the copy-on-fork patch is merged, hence no check for
> >>>>>>> support is added.
> >>>>>>
> >>>>>> Please be more specific, add SHA-1 of that patch and wrote the same
> >>>>>> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
> >>>>>> 1);" line.
> >>>>>>
> >>>>>> Thanks
> >>>>>
> >>>>> Should I put the original commit here? There were quite a lot of bug fixes and
> >>>>> followups that are required.
> >>>>
> >>>> IMHO, the last commit SHA will be enough, the one that has working
> >>>> functionality from your POV.
> >>>
> >>> OK, so that would be:
> >>> 4eae4efa2c29 ("hugetlb: do early cow when page pinned on src mm")
> >>
> >> No, lets put them all
> > 
> > The more data the better chance that it will be missed.
> > It is much saner to add last commit.
> 
> I can gather a list of commits, but I'm not sure I'm aware of every single
> commit that's needed to make this work properly, there's a chance some will be
> missed.

I think it is just the two groups from Peter Xu

Jason

Re: [PATCH for-next] RDMA/nldev: Add copy-on-fork attribute to get sys command

[Gal Pressman]

On 06/04/2021 18:18, Jason Gunthorpe wrote:
> On Tue, Apr 06, 2021 at 05:31:38PM +0300, Gal Pressman wrote:
>> On 06/04/2021 9:44, Leon Romanovsky wrote:
>>> On Mon, Apr 05, 2021 at 07:15:32PM -0300, Jason Gunthorpe wrote:
>>>> On Mon, Apr 05, 2021 at 04:09:39PM +0300, Gal Pressman wrote:
>>>>> On 05/04/2021 15:39, Leon Romanovsky wrote:
>>>>>> On Mon, Apr 05, 2021 at 03:15:18PM +0300, Gal Pressman wrote:
>>>>>>> On 05/04/2021 14:57, Leon Romanovsky wrote:
>>>>>>>> On Mon, Apr 05, 2021 at 02:47:21PM +0300, Gal Pressman wrote:
>>>>>>>>> The new attribute indicates that the kernel copies DMA pages on fork,
>>>>>>>>> hence libibverbs' fork support through madvise and MADV_DONTFORK is not
>>>>>>>>> needed.
>>>>>>>>>
>>>>>>>>> The introduced attribute is always reported as supported since the
>>>>>>>>> kernel has the patch that added the copy-on-fork behavior. This allows
>>>>>>>>> the userspace library to identify older vs newer kernel versions.
>>>>>>>>> Extra care should be taken when backporting this patch as it relies on
>>>>>>>>> the fact that the copy-on-fork patch is merged, hence no check for
>>>>>>>>> support is added.
>>>>>>>>
>>>>>>>> Please be more specific, add SHA-1 of that patch and wrote the same
>>>>>>>> comment near "err = nla_put_u8(msg, RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK,
>>>>>>>> 1);" line.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>
>>>>>>> Should I put the original commit here? There were quite a lot of bug fixes and
>>>>>>> followups that are required.
>>>>>>
>>>>>> IMHO, the last commit SHA will be enough, the one that has working
>>>>>> functionality from your POV.
>>>>>
>>>>> OK, so that would be:
>>>>> 4eae4efa2c29 ("hugetlb: do early cow when page pinned on src mm")
>>>>
>>>> No, lets put them all
>>>
>>> The more data the better chance that it will be missed.
>>> It is much saner to add last commit.
>>
>> I can gather a list of commits, but I'm not sure I'm aware of every single
>> commit that's needed to make this work properly, there's a chance some will be
>> missed.
> 
> I think it is just the two groups from Peter Xu

Will do.