Good idea, will resend. Thanks.
On Wed, 4 May 2022 at 11:08, Paulo Alcantara <pc@xxxxxx> wrote:
>
> Hi Ronnie,
>
> Ronnie Sahlberg <lsahlber@xxxxxxxxxx> writes:
>
> > diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
> > index 1929e80c09ee..8b3fbe6b3580 100644
> > --- a/fs/cifs/readdir.c
> > +++ b/fs/cifs/readdir.c
> > @@ -840,9 +840,112 @@ find_cifs_entry(const unsigned int xid, struct cifs_tcon *tcon, loff_t pos,
> > return rc;
> > }
> >
> > +static bool emit_cached_dirents(struct cached_dirents *cde,
> > + struct dir_context *ctx)
> > +{
> > + struct list_head *tmp;
> > + struct cached_dirent *dirent;
> > + int rc;
> > +
> > + list_for_each(tmp, &cde->entries) {
> > + dirent = list_entry(tmp, struct cached_dirent, entry);
>
> What about list_for_each_entry()?
>
> > +static void add_cached_dirent(struct cached_dirents *cde,
> > + struct dir_context *ctx,
> > + const char *name, int namelen,
> > + struct cifs_fattr *fattr)
> > +{
> > + struct cached_dirent *de;
> > +
> > + if (cde->ctx != ctx)
> > + return;
> > + if (cde->is_valid || cde->is_failed)
> > + return;
> > + if (ctx->pos != cde->pos) {
> > + cde->is_failed = 1;
> > + return;
> > + }
> > + de = kzalloc(sizeof(*de), GFP_ATOMIC);
> > + if (de == NULL) {
> > + cde->is_failed = 1;
> > + return;
> > + }
> > + de->name = kzalloc(namelen + 1, GFP_ATOMIC);
> > + if (de->name == NULL) {
> > + kfree(de);
> > + cde->is_failed = 1;
> > + return;
> > + }
> > + memcpy(de->name, name, namelen);
>
> Replace the above kzalloc() & memcpy() with kstrndup()?
>
> > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > index d6aaeff4a30a..10170266f44b 100644
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -699,6 +699,8 @@ smb2_close_cached_fid(struct kref *ref)
> > {
> > struct cached_fid *cfid = container_of(ref, struct cached_fid,
> > refcount);
> > + struct list_head *pos, *q;
> > + struct cached_dirent *dirent;
> >
> > if (cfid->is_valid) {
> > cifs_dbg(FYI, "clear cached root file handle\n");
> > @@ -718,6 +720,21 @@ smb2_close_cached_fid(struct kref *ref)
> > dput(cfid->dentry);
> > cfid->dentry = NULL;
> > }
> > + /*
> > + * Delete all cached dirent names
> > + */
> > + mutex_lock(&cfid->dirents.de_mutex);
> > + list_for_each_safe(pos, q, &cfid->dirents.entries) {
> > + dirent = list_entry(pos, struct cached_dirent, entry);
>
> list_for_each_entry_safe()?
>
> Nice job! Looks good to me,
>
> Reviewed-by: Paulo Alcantara (SUSE) <pc@xxxxxx>
