| Message ID | alpine.DEB.2.22.394.2501061335161.133435@ubuntu-linux-20-04-desktop (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] xen: update pvcalls_front_accept prototype | expand |
On 06.01.2025 22:36, Stefano Stabellini wrote: > xen: update pvcalls_front_accept prototype > > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> > --- > > Changes in v2: > - also update pvcalls-front.c The patch still gives the impression of being incomplete: There's no caller of the function that you update. However, there's no such caller in the first place. Why don't you just delete the function then? Jan > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c > index b72ee9379d77..cab480059731 100644 > --- a/drivers/xen/pvcalls-front.c > +++ b/drivers/xen/pvcalls-front.c > @@ -769,7 +769,8 @@ int pvcalls_front_listen(struct socket *sock, int backlog) > return ret; > } > > -int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags) > +int pvcalls_front_accept(struct socket *sock, struct socket *newsock, > + struct proto_accept_arg *arg) > { > struct pvcalls_bedata *bedata; > struct sock_mapping *map; > @@ -788,7 +789,7 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags) > return -EINVAL; > } > > - nonblock = flags & SOCK_NONBLOCK; > + nonblock = arg->flags & SOCK_NONBLOCK; > /* > * Backend only supports 1 inflight accept request, will return > * errors for the others > diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h > index f694ad77379f..881ef14660bc 100644 > --- a/drivers/xen/pvcalls-front.h > +++ b/drivers/xen/pvcalls-front.h > @@ -12,7 +12,7 @@ int pvcalls_front_bind(struct socket *sock, > int pvcalls_front_listen(struct socket *sock, int backlog); > int pvcalls_front_accept(struct socket *sock, > struct socket *newsock, > - int flags); > + struct proto_accept_arg *arg); > int pvcalls_front_sendmsg(struct socket *sock, > struct msghdr *msg, > size_t len); >
On Tue, 7 Jan 2025, Jan Beulich wrote: > On 06.01.2025 22:36, Stefano Stabellini wrote: > > xen: update pvcalls_front_accept prototype > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> > > --- > > > > Changes in v2: > > - also update pvcalls-front.c > > The patch still gives the impression of being incomplete: There's no > caller of the function that you update. However, there's no such caller > in the first place. Why don't you just delete the function then? It is meant to be called from an out-of-tree module, which has not been upstreamed yet > > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c > > index b72ee9379d77..cab480059731 100644 > > --- a/drivers/xen/pvcalls-front.c > > +++ b/drivers/xen/pvcalls-front.c > > @@ -769,7 +769,8 @@ int pvcalls_front_listen(struct socket *sock, int backlog) > > return ret; > > } > > > > -int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags) > > +int pvcalls_front_accept(struct socket *sock, struct socket *newsock, > > + struct proto_accept_arg *arg) > > { > > struct pvcalls_bedata *bedata; > > struct sock_mapping *map; > > @@ -788,7 +789,7 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags) > > return -EINVAL; > > } > > > > - nonblock = flags & SOCK_NONBLOCK; > > + nonblock = arg->flags & SOCK_NONBLOCK; > > /* > > * Backend only supports 1 inflight accept request, will return > > * errors for the others > > diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h > > index f694ad77379f..881ef14660bc 100644 > > --- a/drivers/xen/pvcalls-front.h > > +++ b/drivers/xen/pvcalls-front.h > > @@ -12,7 +12,7 @@ int pvcalls_front_bind(struct socket *sock, > > int pvcalls_front_listen(struct socket *sock, int backlog); > > int pvcalls_front_accept(struct socket *sock, > > struct socket *newsock, > > - int flags); > > + struct proto_accept_arg *arg); > > int pvcalls_front_sendmsg(struct socket *sock, > > struct msghdr *msg, > > size_t len); > > >
On 08.01.2025 00:30, Stefano Stabellini wrote: > On Tue, 7 Jan 2025, Jan Beulich wrote: >> On 06.01.2025 22:36, Stefano Stabellini wrote: >>> xen: update pvcalls_front_accept prototype >>> >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> >>> --- >>> >>> Changes in v2: >>> - also update pvcalls-front.c >> >> The patch still gives the impression of being incomplete: There's no >> caller of the function that you update. However, there's no such caller >> in the first place. Why don't you just delete the function then? > > It is meant to be called from an out-of-tree module, which has not been > upstreamed yet And which then would require an EXPORT_SYMBOL() anyway. In Xen, as you're well aware, such unreachable code would actually constitute a Misra violation. Without any in-tree caller, imo the change needs a non-empty description, clarifying why the adjustment is wanted / needed. Jan
On Wed, 8 Jan 2025, Jan Beulich wrote: > On 08.01.2025 00:30, Stefano Stabellini wrote: > > On Tue, 7 Jan 2025, Jan Beulich wrote: > >> On 06.01.2025 22:36, Stefano Stabellini wrote: > >>> xen: update pvcalls_front_accept prototype > >>> > >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> > >>> --- > >>> > >>> Changes in v2: > >>> - also update pvcalls-front.c > >> > >> The patch still gives the impression of being incomplete: There's no > >> caller of the function that you update. However, there's no such caller > >> in the first place. Why don't you just delete the function then? > > > > It is meant to be called from an out-of-tree module, which has not been > > upstreamed yet > > And which then would require an EXPORT_SYMBOL() anyway. In Xen, as you're > well aware, such unreachable code would actually constitute a Misra > violation. > > Without any in-tree caller, imo the change needs a non-empty description, > clarifying why the adjustment is wanted / needed. How about: --- xen: update pvcalls_front_accept prototype While currently there are no in-tree callers of these functions, it is best to keep them up-to-date with the latest network API. Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
On 10.01.2025 01:10, Stefano Stabellini wrote: > On Wed, 8 Jan 2025, Jan Beulich wrote: >> On 08.01.2025 00:30, Stefano Stabellini wrote: >>> On Tue, 7 Jan 2025, Jan Beulich wrote: >>>> On 06.01.2025 22:36, Stefano Stabellini wrote: >>>>> xen: update pvcalls_front_accept prototype >>>>> >>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> >>>>> --- >>>>> >>>>> Changes in v2: >>>>> - also update pvcalls-front.c >>>> >>>> The patch still gives the impression of being incomplete: There's no >>>> caller of the function that you update. However, there's no such caller >>>> in the first place. Why don't you just delete the function then? >>> >>> It is meant to be called from an out-of-tree module, which has not been >>> upstreamed yet >> >> And which then would require an EXPORT_SYMBOL() anyway. In Xen, as you're >> well aware, such unreachable code would actually constitute a Misra >> violation. >> >> Without any in-tree caller, imo the change needs a non-empty description, >> clarifying why the adjustment is wanted / needed. > > How about: > > --- > xen: update pvcalls_front_accept prototype > > While currently there are no in-tree callers of these functions, it is > best to keep them up-to-date with the latest network API. SGTM, fwiw. Jan
diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index b72ee9379d77..cab480059731 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -769,7 +769,8 @@ int pvcalls_front_listen(struct socket *sock, int backlog) return ret; } -int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags) +int pvcalls_front_accept(struct socket *sock, struct socket *newsock, + struct proto_accept_arg *arg) { struct pvcalls_bedata *bedata; struct sock_mapping *map; @@ -788,7 +789,7 @@ int pvcalls_front_accept(struct socket *sock, struct socket *newsock, int flags) return -EINVAL; } - nonblock = flags & SOCK_NONBLOCK; + nonblock = arg->flags & SOCK_NONBLOCK; /* * Backend only supports 1 inflight accept request, will return * errors for the others diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h index f694ad77379f..881ef14660bc 100644 --- a/drivers/xen/pvcalls-front.h +++ b/drivers/xen/pvcalls-front.h @@ -12,7 +12,7 @@ int pvcalls_front_bind(struct socket *sock, int pvcalls_front_listen(struct socket *sock, int backlog); int pvcalls_front_accept(struct socket *sock, struct socket *newsock, - int flags); + struct proto_accept_arg *arg); int pvcalls_front_sendmsg(struct socket *sock, struct msghdr *msg, size_t len);
xen: update pvcalls_front_accept prototype Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> --- Changes in v2: - also update pvcalls-front.c