On Tue, Apr 14, 2020 at 12:53 PM Chengguang Xu <cgxu519@xxxxxxxxxxxx> wrote:
>
> Sharing inode with different whiteout files for saving
> inode and speeding up deleting operation.
>
> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxxxx>
> ---
> v1->v2:
> - Address Amir's comments in v1
>
> v2->v3:
> - Address Amir's comments in v2
> - Rebase on Amir's "Overlayfs use index dir as work dir" patch set
> - Keep at most one whiteout tmpfile in work dir
>
> fs/overlayfs/dir.c | 35 ++++++++++++++++++++++++++++-------
> fs/overlayfs/overlayfs.h | 9 +++++++--
> fs/overlayfs/ovl_entry.h | 4 ++++
> fs/overlayfs/readdir.c | 3 ++-
> fs/overlayfs/super.c | 9 +++++++++
> fs/overlayfs/util.c | 3 ++-
> 6 files changed, 52 insertions(+), 11 deletions(-)
>
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 279009dee366..dbe5e54dcb16 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -62,35 +62,55 @@ struct dentry *ovl_lookup_temp(struct dentry *workdir)
> }
>
> /* caller holds i_mutex on workdir */
> -static struct dentry *ovl_whiteout(struct dentry *workdir)
> +static struct dentry *ovl_whiteout(struct ovl_fs *ofs, struct dentry *workdir)
> {
> int err;
> + bool retried = false;
> + bool should_link = (ofs->whiteout_link_max > 1);
> struct dentry *whiteout;
> struct inode *wdir = workdir->d_inode;
>
> +retry:
> whiteout = ovl_lookup_temp(workdir);
> if (IS_ERR(whiteout))
> return whiteout;
>
> + if (should_link && ofs->whiteout) {
What happens with ofs->whiteout_link_max == 2 is that half of the
times, whiteout gets linked and then unlinked.
That is not needed.
I think code would look better like this:
if (ovl_whiteout_linkable(ofs)) {
err = ovl_do_link(ofs->whiteout, wdir, whiteout);
...
} else if (ofs->whiteout) {
ovl_cleanup(wdir, ofs->whiteout);
....
With:
static inline bool ovl_whiteout_linkable(struct ovl_fs *ofs)
{
return ofs->whiteout &&
ofs->whiteout->d_inode->i_nlink < ofs->whiteout_link_max;
}
> + err = ovl_do_link(ofs->whiteout, wdir, whiteout);
> + if (err || !ovl_whiteout_linkable(ofs)) {
> + ovl_cleanup(wdir, ofs->whiteout);
> + dput(ofs->whiteout);
> + ofs->whiteout = NULL;
> + }
> +
> + if (!err)
> + return whiteout;
> + }
> +
...
> @@ -1762,6 +1767,10 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>
> if (!ofs->workdir)
> sb->s_flags |= SB_RDONLY;
> + else
> + ofs->whiteout_link_max = min_not_zero(
> + ofs->workdir->d_sb->s_max_links,
> + ovl_whiteout_link_max_def ?: 1);
>
Please leave that code inside ovl_get_workdir() or ovl_make_workdir().
Thanks,
Amir.
