On Wed, Apr 11, 2018 at 10:17 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> On Wed, Apr 11, 2018 at 7:41 AM, cgxu519@xxxxxxx <cgxu519@xxxxxxx> wrote:
>>
>>> 在 2018年4月10日，下午1:38，Amir Goldstein <amir73il@xxxxxxxxx> 写道：
>>>
>>> On Tue, Apr 10, 2018 at 4:29 AM, cgxu519@xxxxxxx <cgxu519@xxxxxxx> wrote:
>>>> Hi guys,
>>>>
>>>> I’m a little curious for below code in ovl_lookup, is there any background for
>>>> copy stack instead of directly using it?
>>>>
>>>> memcpy(oe->lowerstack, stack, sizeof(struct ovl_path) * ctr);
>>>>
>>>
>>> Memory compaction.
>>>
>>> stack has ofs->numlower entries, which could be very large,
>>> but after lookup, oe->numlower can end up being much much
>>> smaller. Also, ovl_alloc_entry() packs lowerstack together with
>>> ovl_entry, so the overall memory usage of the long lived overlay
>>> dentry cache entries is lower.
>>
>> For regular file the maximum numlower is 1, so for optimizing memory
>> allocation/free maybe we can put one ovl_path entry inside ovl_entry
>> and allocate/deallocate ovl_entry via cache pool and for dir we can
>> link to ovl_path array from ovl_entry. Does it make sense?
>
> Yes. We can do it for dir as well if there's only one lower layer
> (it's going to be the common case).
>
You can do this for dir not only if ofs->numlower == 1, but also if
oe->numlower == 1 and decide how to free oe depending on oe->numlower.
Pre-allocating one ovl_path is waste for pure upper entries, but I guess
that is the less common case(?).
The size of allocated ovl_entry for non-dir or merge dir with single lower
layer on 64bit machine is 40 bytes, which means quite a big waste with
kmalloc (24 bytes), so cache pool makes sense.
We could pack the ovl_entry further to 32 bytes with or without cache
pool. Not sure if it is worse it though(?):
struct ovl_path {
unsigned short reserved;
unsigned short layer_idx;
struct dentry *dentry;
};
struct ovl_entry {
union {
struct {
unsigned long flags;
};
struct rcu_head rcu;
};
union {
unsigned short numlower;
struct ovl_path lowerstack[1];
};
};
And for example:
static inline struct ovl_layer *ovl_dentry_layer(struct dentry *dentry, int idx)
{
struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
struct ovl_entry *oe = dentry->d_fsdata;
return idx ? &ofs->layer_layers[oe->lowerstack[idx -
1]->layer_idx - 1] : NULL;
}
Thanks,
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
