Re: [PATCH v5 09/21] nfsd: add data structures for handling CB_NOTIFY

Jeff Layton <[email protected]>
Newsgroups gmane.linux.nfs,gmane.linux.kernel,gmane.linux.documentation
Message-ID <[email protected]>
On Mon, 2026-06-08 at 16:18 -0400, Chuck Lever wrote:
> 
> On Fri, May 22, 2026, at 3:42 PM, Jeff Layton wrote:
> > Add the data structures, allocation helpers, and callback operations
> > needed for directory delegation CB_NOTIFY support:
> 
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index 9c6e2e7abc82..505fabf8f1bf 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -197,6 +197,44 @@ struct nfs4_cb_fattr {
> >  #define NOTIFY4_EVENT_QUEUE_SIZE	3
> >  #define NOTIFY4_PAGE_ARRAY_SIZE		1
> > 
> > +struct nfsd_notify_event {
> > +	refcount_t	ne_ref;		// refcount
> > +	u32		ne_mask;	// FS_* mask from fsnotify callback
> > +	struct dentry	*ne_dentry;	// dentry reference to target
> > +	u32		ne_namelen;	// length of ne_name
> > +	char		ne_name[];	// name of dentry being changed
> 
> Nit: checkpatch doesn't like the C++ comment style.
> 
> 
> > +};
> > +
> > +static inline struct nfsd_notify_event *nfsd_notify_event_get(struct 
> > nfsd_notify_event *ne)
> > +{
> > +	refcount_inc(&ne->ne_ref);
> > +	return ne;
> > +}
> > +
> > +static inline void nfsd_notify_event_put(struct nfsd_notify_event *ne)
> > +{
> > +	if (refcount_dec_and_test(&ne->ne_ref)) {
> > +		dput(ne->ne_dentry);
> > +		kfree(ne);
> > +	}
> > +}
> > +
> > +/*
> > + * Represents a directory delegation. The callback is for handling 
> > CB_NOTIFYs.
> > + * As notifications from fsnotify come in, allocate a new event, take 
> > the ncn_lock,
> > + * and add it to the ncn_evt queue. The CB_NOTIFY prepare handler will 
> > take the
> > + * lock, clean out the list and process it.
> > + */
> > +struct nfsd4_cb_notify {
> > +	spinlock_t			ncn_lock;	// protects the evt queue and count
> > +	int				ncn_evt_cnt;	// count of events in ncn_evt
> > +	int				ncn_nf_cnt;	// count of valid entries in ncn_nf
> > +	struct nfsd_notify_event	*ncn_evt[NOTIFY4_EVENT_QUEUE_SIZE]; // list 
> > of events
> > +	struct page			*ncn_pages[NOTIFY4_PAGE_ARRAY_SIZE]; // for encoding
> > +	struct notify4			*ncn_nf;	// array of notify4's to be sent
> > +	struct nfsd4_callback		ncn_cb;		// notify4 callback
> > +};
> 
> Ditto.
> 


I'll note that the code is littered with this comment style anyway,
including a bunch of the new // SPDX- header comments. Personally, I
find this more readable for documenting struct fields.

AFAICT, the checkpatch rule was manufactured out of thin air.
Documentation/dev-tools/checkpatch.rst says:

  **C99_COMMENTS**
    C99 style single line comments (//) should not be used.
    Prefer the block comment style instead.

    See:
https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting

...but that coding-style document says nothing about C99 comments. I
move that we ignore checkpatch here.
-- 
Jeff Layton <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.