On Sat, 2024-08-24 at 14:21 -0400, Chuck Lever wrote: > On Sat, Aug 24, 2024 at 07:07:50PM +0100, Simon Horman wrote: > > On Thu, Aug 22, 2024 at 09:39:08PM +0800, Li Zetao wrote: > > > When reading pages in xdr_read_pages, the number of XDR encoded > > > bytes > > > should be less than the len of aligned pages, so using min() here > > > is > > > very semantic. > > > > > > Signed-off-by: Li Zetao <lizetao1@xxxxxxxxxx> > > > --- > > > net/sunrpc/xdr.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c > > > index 62e07c330a66..6746e920dbbb 100644 > > > --- a/net/sunrpc/xdr.c > > > +++ b/net/sunrpc/xdr.c > > > @@ -1602,7 +1602,7 @@ unsigned int xdr_read_pages(struct > > > xdr_stream *xdr, unsigned int len) > > > end = xdr_stream_remaining(xdr) - pglen; > > > > > > xdr_set_tail_base(xdr, base, end); > > > - return len <= pglen ? len : pglen; > > > + return min(len, pglen); > > > > Both len and pglen are unsigned int, so this seems correct to me. > > > > And the code being replaced does appear to be a min() operation in > > both form and function. > > > > Reviewed-by: Simon Horman <horms@xxxxxxxxxx> > > > > However, I don't believe SUNRPC changes usually don't go through > > next-next. > > So I think this either needs to be reposted or get some acks from > > Chuck Lever (already CCed). > > > > Chuck, perhaps you can offer some guidance here? > > > > > } > > > EXPORT_SYMBOL_GPL(xdr_read_pages); > > > > > > -- > > > 2.34.1 > > > > > > > > Changes to net/sunrpc/ can go through Anna and Trond's NFS client > trees, through the NFSD tree, or via netdev, but they are typically > taken through the NFS-related trees. > > Unless the submitter or the relevant maintainers prefer otherwise, > I don't see a problem with this one going through netdev. Let me > know otherwise. > > Acked-by: Chuck Lever <chuck.lever@xxxxxxxxxx> > > What is the value of this change? Unless the current code is actually broken or somehow difficult to read, then I much prefer to leave it untouched so that any future back ports of fixes to code around that line remain trivial. So NACK to this change for now. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
