Re: [PATCH v1 3/3] svcrdma: Fix leak of svc_rdma_recv_ctxt objects

[Jason Gunthorpe <jgg@ziepe.ca>]

On Thu, Apr 09, 2020 at 10:33:32AM -0400, Chuck Lever wrote:
> Hi Leon-
> 
> > On Apr 8, 2020, at 2:02 AM, Leon Romanovsky <leon@kernel.org> wrote:
> > 
> > On Tue, Apr 07, 2020 at 03:11:06PM -0400, Chuck Lever wrote:
> >> Utilize the xpo_release_rqst transport method to ensure that each
> >> rqstp's svc_rdma_recv_ctxt object is released even when the server
> >> cannot return a Reply for that rqstp.
> >> 
> >> Without this fix, each RPC whose Reply cannot be sent leaks one
> >> svc_rdma_recv_ctxt. This is a 2.5KB structure, a 4KB DMA-mapped
> >> Receive buffer, and any pages that might be part of the Reply
> >> message.
> >> 
> >> The leak is infrequent unless the network fabric is unreliable or
> >> Kerberos is in use, as GSS sequence window overruns, which result
> >> in connection loss, are more common on fast transports.
> >> 
> >> Fixes: 3a88092ee319 ("svcrdma: Preserve Receive buffer until ... ")
> > 
> > Chuck,
> > 
> > Can you please don't mangle the Fixes line?
> 
> I've read e-mail from others that advocate this form of mangling
> instead of using commit message lines that are too long.

Really? 

At least I won't accept Fixes lines that are not in the cannonical
format, I routinely fix these things in all sorts of ways, but I've
never seen someone shorten it with ...

> > A lot of automatization is relying on the fact that this line is canonical,
> > both in format and in the actual content.
> 
> Understood, but checkpatch.pl does not complain about it. Perhaps,
> therefore, it is the automation that is not correct.

checkpatch.pl doesn't check Fixes lines for correctness, because it
doesn't have access to the git or something. This was talked about
too.. Stephen likes to check them as part of linux-next though.

However, checkpatch.pl does not complain for long lines on Fixes:
tags demanding they be shortened

> The commit ID is what automation should key off of. The short
> description is only for human consumption. 

Right, so if the actual commit message isn't included so humans can
read it then what was the point of including anything?

Jason