On Thu, Oct 28, 2010 at 11:50 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Thu, 28 Oct 2010 12:23:25 -0400
> Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
>> On Thu, 28 Oct 2010 10:06:05 -0500
>> Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>>
>> > On Thu, Oct 28, 2010 at 6:33 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>> > > Instead of keeping a tag on the master tlink in the tree, just keep a
>> > > pointer to the master in the superblock. That eliminates the need for
>> > > using the radix tree to look up a tagged entry.
>> > >
>> > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>> > > ---
>> > > fs/cifs/cifs_fs_sb.h | 2 +-
>> > > fs/cifs/connect.c | 18 +++---------------
>> > > 2 files changed, 4 insertions(+), 16 deletions(-)
>> > >
>> > > diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
>> > > index 525ba59..79576da 100644
>> > > --- a/fs/cifs/cifs_fs_sb.h
>> > > +++ b/fs/cifs/cifs_fs_sb.h
>> > > @@ -43,8 +43,8 @@
>> > >
>> > > struct cifs_sb_info {
>> > > struct radix_tree_root tlink_tree;
>> > > -#define CIFS_TLINK_MASTER_TAG 0 /* is "master" (mount) tcon */
>> > > spinlock_t tlink_tree_lock;
>> > > + struct tcon_link *master_tlink;
>> > > struct nls_table *local_nls;
>> > > unsigned int rsize;
>> > > unsigned int wsize;
>> > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > > index 469c3dd..364eb96 100644
>> > > --- a/fs/cifs/connect.c
>> > > +++ b/fs/cifs/connect.c
>> > > @@ -2917,11 +2917,11 @@ remote_path_check:
>> > >
>> > > spin_lock(&cifs_sb->tlink_tree_lock);
>> > > radix_tree_insert(&cifs_sb->tlink_tree, pSesInfo->linux_uid, tlink);
>> > > - radix_tree_tag_set(&cifs_sb->tlink_tree, pSesInfo->linux_uid,
>> > > - CIFS_TLINK_MASTER_TAG);
>> > > spin_unlock(&cifs_sb->tlink_tree_lock);
>> > > radix_tree_preload_end();
>> > >
>> > > + cifs_sb->master_tlink = tlink;
>> > > +
>> > > queue_delayed_work(system_nrt_wq, &cifs_sb->prune_tlinks,
>> > > TLINK_IDLE_EXPIRE);
>> > >
>> > > @@ -3275,19 +3275,7 @@ out:
>> > > static struct tcon_link *
>> > > cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb)
>> > > {
>> > > - struct tcon_link *tlink;
>> > > - unsigned int ret;
>> > > -
>> > > - spin_lock(&cifs_sb->tlink_tree_lock);
>> > > - ret = radix_tree_gang_lookup_tag(&cifs_sb->tlink_tree, (void **)&tlink,
>> > > - 0, 1, CIFS_TLINK_MASTER_TAG);
>> > > - spin_unlock(&cifs_sb->tlink_tree_lock);
>> > > -
>> > > - /* the master tcon should always be present */
>> > > - if (ret == 0)
>> > > - BUG();
>> > > -
>> > > - return tlink;
>> > > + return cifs_sb->master_tlink;
>> >
>> > Wondering whether we need a function just to return master_tlink
>> > within a cifs_sb.
>> > Could the caller(s) of cifs_sb_master_tlink() access master_tlink directly?
>> >
>>
>> Sure, but accessor functions are generally a good thing. It insulates
>> the callers from needing to know anything about the underlying data
>> structure.
>>
>
> What may make some sense however is turning this function into a static
> inline or macro. Thoughts?
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
>
I think inline function would be my preferred choice.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
