On Thu, Jul 6, 2023 at 8:51 AM Steve French <smfrench@xxxxxxxxx> wrote:
>
> updated to fix two checkpatch warnings (see attached) and tentatively
> merged into for-next pending review/testing
>
>
> On Wed, Jul 5, 2023 at 9:32 PM Ronnie Sahlberg <lsahlber@xxxxxxxxxx> wrote:
> >
> > and drop cached directories after 30 seconds
> >
> > Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> > ---
> > fs/smb/client/cached_dir.c | 67 ++++++++++++++++++++++++++++++++++++++
> > fs/smb/client/cached_dir.h | 1 +
> > 2 files changed, 68 insertions(+)
> >
> > diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
> > index bfc964b36c72..f567eea0a456 100644
> > --- a/fs/smb/client/cached_dir.c
> > +++ b/fs/smb/client/cached_dir.c
> > @@ -568,6 +568,53 @@ static void free_cached_dir(struct cached_fid *cfid)
> > kfree(cfid);
> > }
> >
> > +static int
> > +cifs_cfids_laundromat_thread(void *p)
> > +{
> > + struct cached_fids *cfids = p;
> > + struct cached_fid *cfid, *q;
> > + struct list_head entry;
> > +
> > + while (!kthread_should_stop()) {
> > + ssleep(1);
> > + INIT_LIST_HEAD(&entry);
> > + if (kthread_should_stop())
> > + return 0;
> > + spin_lock(&cfids->cfid_list_lock);
> > + list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
> > + if (jiffies > cfid->time + HZ * 30) {
> > + list_del(&cfid->entry);
> > + list_add(&cfid->entry, &entry);
> > + cfids->num_entries--;
> > + }
> > + }
> > + spin_unlock(&cfids->cfid_list_lock);
> > +
> > + list_for_each_entry_safe(cfid, q, &entry, entry) {
> > + cfid->on_list = false;
> > + list_del(&cfid->entry);
> > + /*
> > + * Cancel, and wait for the work to finish in
> > + * case we are racing with it.
> > + */
> > + cancel_work_sync(&cfid->lease_break);
> > + if (cfid->has_lease) {
> > + /*
> > + * We lease has not yet been cancelled from
> > + * the server so we need to drop the reference.
> > + */
> > + spin_lock(&cfids->cfid_list_lock);
> > + cfid->has_lease = false;
> > + spin_unlock(&cfids->cfid_list_lock);
> > + kref_put(&cfid->refcount, smb2_close_cached_fid);
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +
> > struct cached_fids *init_cached_dirs(void)
> > {
> > struct cached_fids *cfids;
> > @@ -577,6 +624,20 @@ struct cached_fids *init_cached_dirs(void)
> > return NULL;
> > spin_lock_init(&cfids->cfid_list_lock);
> > INIT_LIST_HEAD(&cfids->entries);
> > +
> > + /*
> > + * since we're in a cifs function already, we know that
> > + * this will succeed. No need for try_module_get().
> > + */
> > + __module_get(THIS_MODULE);
> > + cfids->laundromat = kthread_run(cifs_cfids_laundromat_thread,
> > + cfids, "cifsd-cfid-laundromat");
> > + if (IS_ERR(cfids->laundromat)) {
> > + cifs_dbg(VFS, "Failed to start cfids laundromat thread.\n");
> > + kfree(cfids);
> > + module_put(THIS_MODULE);
> > + return NULL;
> > + }
> > return cfids;
> > }
> >
> > @@ -589,6 +650,12 @@ void free_cached_dirs(struct cached_fids *cfids)
> > struct cached_fid *cfid, *q;
> > LIST_HEAD(entry);
> >
> > + if (cfids->laundromat) {
> > + kthread_stop(cfids->laundromat);
> > + cfids->laundromat = NULL;
> > + module_put(THIS_MODULE);
> > + }
> > +
> > spin_lock(&cfids->cfid_list_lock);
> > list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
> > cfid->on_list = false;
> > diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
> > index 2f4e764c9ca9..facc9b154d00 100644
> > --- a/fs/smb/client/cached_dir.h
> > +++ b/fs/smb/client/cached_dir.h
> > @@ -57,6 +57,7 @@ struct cached_fids {
> > spinlock_t cfid_list_lock;
> > int num_entries;
> > struct list_head entries;
> > + struct task_struct *laundromat;
> > };
> >
> > extern struct cached_fids *init_cached_dirs(void);
> > --
> > 2.35.3
> >
>
>
> --
> Thanks,
>
> Steve
Hi Ronnie,
Why do we need this?
We only cache dirents if we have the parent directory lease. As long
as we have the lease, why can't we continue to cache these?
--
Regards,
Shyam
