overlayfs: Delayed copy up of data

Right now my understanding is that origin xattr is created for all copied up files if index=on. And if index=off, then we create it for all type of files except hardlinks (nlink != 1).

With metadata only copy up, I will still require origin xattr to copy up data later, so create it even for hardlinks even with index=off.

Given ->origin is always true now, get rid of this field from "struct ovl_copy_up_ctx".

Signed-off-by Vivek Goyal <[email protected]>

By default metadata only copy up is disabled. Provide a mount option so that users can choose one way or other.

Also provide a kernel config and module option to enable/disable metacopy feature.

Like index feature, we verify on mount that upper root is not being reused with a different lower root. This hopes to get the configuration right and detect the copied layers use case. But this does only so much as we don't verify all the lowers. So it is possible that a lower is missing and later data copy up fails.

Signed-off-by Vivek Goyal <[email protected]>

fs/overlayfs/super.c +2 -1

-       if (ofs->config.index && !ovl_can_decode_fh(path->dentry->d_sb)) {
+       if ((ofs->config.index || ofs->config.metacopy) &&
+            !ovl_can_decode_fh(path->dentry->d_sb)) {

Quoted 6 lines · Vivek Goyal

By default metadata only copy up is disabled. Provide a mount option so that users can choose one way or other.

Also provide a kernel config and module option to enable/disable metacopy feature.

Quoted 7 lines

Like index feature, we verify on mount that upper root is not being reused with a different lower root. This hopes to get the configuration right and detect the copied layers use case. But this does only so much as we don't verify all the lowers. So it is possible that a lower is missing and later data copy up fails.

Signed-off-by Vivek Goyal <[email protected]>

Fine, as long as Miklos agrees not to enforce exclusive dir lock, it's fine by me.

Please take a look at my ovl-features patches and say what you think. It is time we started to enforce some order with all the inter-dependencies between features.
With my proposal, it will make mounting an overlay with metacopy object on older kernel slightly harder (but not impossible), so it can help admins from tripping over themselves.

Reviewed-by Amir Goldstein <[email protected]>

Quoted 18 lines · Amir Goldstein

Quoted 6 lines · Vivek Goyal

By default metadata only copy up is disabled. Provide a mount option so that users can choose one way or other.

Also provide a kernel config and module option to enable/disable metacopy feature.

Quoted 7 lines

Like index feature, we verify on mount that upper root is not being reused with a different lower root. This hopes to get the configuration right and detect the copied layers use case. But this does only so much as we don't verify all the lowers. So it is possible that a lower is missing and later data copy up fails.

Signed-off-by Vivek Goyal <[email protected]>

Fine, as long as Miklos agrees not to enforce exclusive dir lock, it's fine by me.

Quoted 9 lines

Please take a look at my ovl-features patches and say what you think. It is time we started to enforce some order with all the inter-dependencies between features.
With my proposal, it will make mounting an overlay with metacopy object on older kernel slightly harder (but not impossible), so it can help admins from tripping over themselves.

Reviewed-by Amir Goldstein <[email protected]>

On second though, hold that Reviewed-by, there is one more thing that this patch should check, see path:
"ovl: disable redirect_dir and index when no xattr support" from my verify_dir patches:
https://github.com/amir73il/linux/commits/ovl-verify-dir

You may cherry-pick it at beginning of your series and then this patch should also disable metacopy on mount with noxattr with a warning to user, instead of silently not doing the metacopies, but claiming in mount options that metacopy is on.

Quoted 6 lines · Vivek Goyal

By default metadata only copy up is disabled. Provide a mount option so that users can choose one way or other.

Also provide a kernel config and module option to enable/disable metacopy feature.

Quoted 7 lines

Like index feature, we verify on mount that upper root is not being reused with a different lower root. This hopes to get the configuration right and detect the copied layers use case. But this does only so much as we don't verify all the lowers. So it is possible that a lower is missing and later data copy up fails.

Signed-off-by Vivek Goyal <[email protected]>

Fine, as long as Miklos agrees not to enforce exclusive dir lock, it's fine by me.

Please take a look at my ovl-features patches and say what you think. It is time we started to enforce some order with all the inter-dependencies between features.
With my proposal, it will make mounting an overlay with metacopy object on older kernel slightly harder (but not impossible), so it can help admins from tripping over themselves.

Quoted 4 lines

fs/overlayfs/super.c +2 -1

-       if (ofs->config.index && !ovl_can_decode_fh(path->dentry->d_sb)) {
+       if ((ofs->config.index || ofs->config.metacopy) &&
+            !ovl_can_decode_fh(path->dentry->d_sb)) {

fs/overlayfs/super.c static int ovl_lower_dir(const char *name, struct path *path, · line 690

Quoted 10 lines

fs/overlayfs/super.c +7 -1

+               if (ofs->config.index)
+                       pr_warn("overlayfs: fs on '%s' does not support file handles, falling back to index=off.\n", name);
+
+               if (ofs->config.metacopy)
+                       pr_warn("overlayfs: fs on '%s' does not support file handles, falling back to metacopy=off.\n", name);
+
                ofs->config.index = false;
-               pr_warn("overlayfs: fs on '%s' does not support file handles, falling back to index=off.\n", name);
+               ofs->config.metacopy = false;
        }

So you didn't like the less granular but shorter warning I used in my verify_dir patches, e.g.:

                 ofs->config.index = false;

- pr_warn("overlayfs: fs on '%s' does not support file
handles, falling back to index=off.\n", name);

(quoted diff) +2 -0

+               ofs->config.metacopy = false;
+               pr_warn("overlayfs: fs on '%s' does not support file

handles, falling back to index=off,metacopy=off.\n", name);

Doesn't matter much to me,

If it makes sense to copy up only metadata during copy up, do it. This is done for regular files which are not opened for WRITE and have origin being saved.

Right now ->metacopy is set to 0 always. Last patch in the series will remove the hard coded statement and enable metacopy feature.

Signed-off-by Vivek Goyal <[email protected]>

---
 fs/overlayfs/copy_up.c   | 18 +++++++++++-------
 fs/overlayfs/overlayfs.h |  1 +
 fs/overlayfs/util.c      | 22 ++++++++++++++++++++++
 3 files changed, 34 insertions(+), 7 deletions(-)

fs/overlayfs/copy_up.c +11 -7

index 303794bb9127..6bb6b1723564 100644
@@ -305,11 +305,8 @@ static int ovl_set_origin(struct dentry *dentry, struct dentry *lower,
                        return PTR_ERR(fh);
        }
 
-       /*
-        * Do not fail when upper doesn't support xattrs.
-        */
        err = ovl_check_setxattr(dentry, upper, OVL_XATTR_ORIGIN, fh,
-                                fh ? fh->len : 0, 0);
+                                fh ? fh->len : 0, -EOPNOTSUPP);
        kfree(fh);
 
        return err;

@@ -326,6 +323,7 @@ struct ovl_copy_up_ctx {
        struct qstr destname;
        struct dentry *workdir;
        bool tmpfile;
+       bool metacopy;
 };
 
 static int ovl_link_up(struct ovl_copy_up_ctx *c)

@@ -453,10 +451,14 @@ static int ovl_copy_up_inode(struct ovl_copy_up_ctx *c, struct dentry *temp)
         *
         */
        err = ovl_set_origin(c->dentry, c->lowerpath.dentry, temp);
-       if (err)
-               return err;
+       if (err) {
+               if (err != -EOPNOTSUPP)
+                       return err;
+               /* Upper does not support xattr. Copy up data as well */
+               c->metacopy = false;
+       }
 
-       if (S_ISREG(c->stat.mode)) {
+       if (S_ISREG(c->stat.mode) && !c->metacopy) {
                struct path upperpath;
 
                ovl_path_upper(c->dentry, &upperpath);

@@ -601,6 +603,8 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
        if (err)
                return err;
 
+       ctx.metacopy = ovl_metaonly_copy_up(dentry, ctx.stat.mode, flags);
+
        ovl_path_upper(parent, &parentpath);
        ctx.destdir = parentpath.dentry;
        ctx.destname = dentry->d_name;

fs/overlayfs/overlayfs.h +1 -0

index 13eab09a6b6f..9e5aaa4a5db2 100644
@@ -215,6 +215,7 @@ bool ovl_dentry_is_whiteout(struct dentry *dentry);
 void ovl_dentry_set_opaque(struct dentry *dentry);
 bool ovl_dentry_has_upper_alias(struct dentry *dentry);
 void ovl_dentry_set_upper_alias(struct dentry *dentry);
+bool ovl_metaonly_copy_up(struct dentry *dentry, umode_t mode, int flags);
 bool ovl_redirect_dir(struct super_block *sb);
 const char *ovl_dentry_get_redirect(struct dentry *dentry);
 void ovl_dentry_set_redirect(struct dentry *dentry, const char *redirect);

fs/overlayfs/util.c +0 -0

index 77dc00438d54..c5b62546c613 100644

fs/overlayfs/util.c +8 -0

+{
+       struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
+
+       /* TODO: Will enable metacopy in last patch of series */
+       return false;
+
+       if (!ofs->config.metacopy)
+               return false;

There are couple of places where we need to know if file is already copied up (in lockless manner). Right now its open coded and there are only two conditions to check. Soon this patch series will introduce another condition to check and Amir wants to introduce one more. So introduce a helper instead to check this so that code is easier to read.

Signed-off-by Vivek Goyal <[email protected]>

Now we will have the capability to have upper inodes which might be only metadata copy up and data is still on lower inode. So add a new xattr OVL_XATTR_METACOPY to distinguish between two cases.

Presence of OVL_XATTR_METACOPY reflects that file has been copied up metadata only and and data will be copied up later from lower origin.
So this xattr is set when a metadata copy takes place and cleared when data copy takes place.

We also use a bit in ovl_inode->flags to cache OVL_UPPERDATA which reflects whether ovl inode has data or not (as opposed to metadata only copy up).

Note, OVL_UPPERDATA is set only on those upper inodes which can possibly be metacopy only inodes. For example, inode should be a regular file and there needs to be a lower dentry.

If an inode has been copied up metadata only, then we need to query the number of blocks from lower and fill up the stat->st_blocks.

Signed-off-by Vivek Goyal <[email protected]>

---
 fs/overlayfs/inode.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

fs/overlayfs/inode.c +6 -1

index 405a8c34471b..7ba19a97a8da 100644
@@ -76,6 +76,10 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
        bool is_dir = S_ISDIR(dentry->d_inode->i_mode);
        bool samefs = ovl_same_sb(dentry->d_sb);
        int err;
+       bool metacopy = false;
+
+       if (ovl_dentry_upper(dentry) && !ovl_has_upperdata(dentry))
+               metacopy = true;
 
        type = ovl_path_real(dentry, &realpath);
        old_cred = ovl_override_creds(dentry->d_sb);

@@ -93,7 +97,8 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
        if (!is_dir || samefs) {
                if (OVL_TYPE_ORIGIN(type)) {
                        struct kstat lowerstat;
-                       u32 lowermask = STATX_INO | (!is_dir ? STATX_NLINK : 0);
+                       u32 lowermask = STATX_INO | STATX_BLOCKS |
+                                       (!is_dir ? STATX_NLINK : 0);
 
                        ovl_path_lower(dentry, &realpath);
                        err = vfs_getattr(&realpath, &lowerstat,

Quoted 31 lines · Vivek Goyal

If an inode has been copied up metadata only, then we need to query the number of blocks from lower and fill up the stat->st_blocks.

Signed-off-by Vivek Goyal <[email protected]>

---
 fs/overlayfs/inode.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

fs/overlayfs/inode.c +6 -1

index 405a8c34471b..7ba19a97a8da 100644
@@ -76,6 +76,10 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
        bool is_dir = S_ISDIR(dentry->d_inode->i_mode);
        bool samefs = ovl_same_sb(dentry->d_sb);
        int err;
+       bool metacopy = false;
+
+       if (ovl_dentry_upper(dentry) && !ovl_has_upperdata(dentry))
+               metacopy = true;
 
        type = ovl_path_real(dentry, &realpath);
        old_cred = ovl_override_creds(dentry->d_sb);

@@ -93,7 +97,8 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
        if (!is_dir || samefs) {
                if (OVL_TYPE_ORIGIN(type)) {
                        struct kstat lowerstat;
-                       u32 lowermask = STATX_INO | (!is_dir ? STATX_NLINK : 0);
+                       u32 lowermask = STATX_INO | STATX_BLOCKS |
+                                       (!is_dir ? STATX_NLINK : 0);
 

fs/overlayfs/inode.c int ovl_getattr(const struct path *path, struct kstat *stat, · line 127

Quoted 9 lines

fs/overlayfs/inode.c +3 -0

                        ovl_path_lower(dentry, &realpath);
                        err = vfs_getattr(&realpath, &lowerstat,

@@ -117,6 +122,9 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
                                WARN_ON_ONCE(stat->dev != lowerstat.dev);
                        else
                                stat->dev = ovl_get_pseudo_dev(dentry);
+
+                       if (metacopy)
+                               stat->blocks = lowerstat.blocks;

OVL_TYPE_ORIGIN(type) implies ovl_dentry_upper(dentry), so you don't need the boolean
var, you can test !ovl_has_upperdata(dentry) here instead.

To be honest, I am not fond of the patches adding memory barriers being separate from
that patches that introduce the races, even though technically races cannot happen
until last patch enables metacopy.

During lookup, check for presence of OVL_XATTR_METACOPY and if not present, set OVL_UPPERDATA bit in flags. This is only done for upper inode with a corresponding lower dentry and file needs to be a regular file. Basically any file which is eligible for metadata only copy up.

Signed-off-by Vivek Goyal <[email protected]>

---
 fs/overlayfs/namei.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

fs/overlayfs/namei.c +37 -0

index 625ed8066570..74c15d001d68 100644
@@ -25,6 +25,28 @@ struct ovl_lookup_data {
        char *redirect;
 };
 
+/* err < 0, 0 if no metacopy xattr, 1 if metacopy xattr found */
+static int ovl_check_metacopy(struct dentry *dentry)
+{
+       int res;
+
+       /* Only regular files can have metacopy xattr */
+       if (!S_ISREG(d_inode(dentry)->i_mode))
+               return 0;
+
+       res = vfs_getxattr(dentry, OVL_XATTR_METACOPY, NULL, 0);
+       if (res < 0) {
+               if (res == -ENODATA || res == -EOPNOTSUPP)
+                       return 0;
+               goto out;
+       }
+
+       return 1;
+out:
+       pr_warn_ratelimited("overlayfs: failed to get metacopy (%i)\n", res);
+       return res;
+}
+
 static int ovl_check_redirect(struct dentry *dentry, struct ovl_lookup_data *d,
                              size_t prelen, const char *post)
 {

@@ -602,6 +624,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
        struct dentry *this;
        unsigned int i;
        int err;
+       bool metacopy = false;
        struct ovl_lookup_data d = {
                .name = dentry->d_name,
                .is_dir = false,

@@ -642,6 +665,20 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
                                               roe->numlower, &stack, &ctr);
                        if (err)
                                goto out_put_upper;
+
+                       err = ovl_check_metacopy(upperdentry);
+                       metacopy = err;
+                       if (err < 0)
+                               goto out_put_upper;
+                       if (metacopy && !ctr) {
+                               /*
+                                * Found an upper with metacopy set but
+                                * at the same time there is no lower
+                                * dentry. Something is not right.
+                                */
+                               err = -ESTALE;
+                               goto out_put_upper;
+                       }
                }
 
                if (d.redirect) {

Upper dentry inode does not have data. So return lower dentry if upper is only a metadata copy.

Signed-off-by Vivek Goyal <[email protected]>

---
 fs/overlayfs/super.c | 5 +++++
 1 file changed, 5 insertions(+)

fs/overlayfs/super.c +0 -0

index 84b8f6dc0f61..5cb19781763d 100644

d_real() can make a upper metacopy dentry/inode visible to the vfs layer. This is something new and vfs layer does not know that this inode contains only metadata and not data. And this could break things.

So to be safe, do not export metacopy only dentry/inode to vfs using d_real().

If d_real() is called with flag D_REAL_UPPER, return upper dentry only if it has data (flag OVL_UPPERDATA is set).

Similiarly, if d_real(inode=X) is called, a warning is emitted if returned dentry/inode does not have OVL_UPPERDATA set. This should not happen as we never made this metacopy inode visible to vfs so nobody should be calling overlayfs back with inode=metacopy_inode.

I scanned the code and I don't think it breaks any of the existing code. There are two users of D_REAL_UPPER. may_write_real() and update_ovl_inode_times().

may_write_real(), will get an NULL dentry if upper inode is metacopy only and it will return -EPERM. Effectively, we are disallowing modifications to metacopy only inode from this interface. Though there is opportunity to improve it. (Allow chattr on metacopy inodes).

update_ovl_inode_times() gets inode mtime and ctime from real inode. It should not be broken for metacopy inode as well for following reasons.

- For any metadata operations (setattr, acl etc), overlay always calls ovl_copyattr() and updates ovl inode mtime and ctime. So there is no need to update mtime and ctime int his case. Its already updated.

- For metadata inode, mtime should be same as lower and not change. (data can't be modified on metadata inode without copyup).

- For file writes, ctime and mtime will be updated. But in that case first data will be copied up and this will not be a metadata inode anymore. And furthr call to d_real(D_REAL_UPPER) will return upper inode and new mtime and ctime will be obtainable.

So atime updates should work just fine for metacopy inodes.

Signed-off-by Vivek Goyal <[email protected]>

---
 fs/overlayfs/super.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

fs/overlayfs/super.c +8 -2

index 5cb19781763d..f1d7d38b747f 100644
@@ -84,8 +84,14 @@ static struct dentry *ovl_d_real(struct dentry *dentry,
        struct dentry *real;
        int err;
 
-       if (flags & D_REAL_UPPER)
-               return ovl_dentry_upper(dentry);
+       if (flags & D_REAL_UPPER) {
+               real = ovl_dentry_upper(dentry);
+               if (!real)
+                       return NULL;
+               if (!ovl_has_upperdata(dentry))
+                       return NULL;
+               return real;
+       }
 
        if (!d_is_reg(dentry)) {
                if (!inode || inode == d_inode(dentry))

If file is metacopy only, it is possible that lower is encrypted while other is not. In that case, report file as encrypted (despite the fact that only data is encrypted while metadata is not).

Signed-off-by Vivek Goyal <[email protected]>

---
 fs/overlayfs/inode.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

fs/overlayfs/inode.c +0 -0

index 7ba19a97a8da..15713d4ac2dd 100644
@@ -66,6 +66,16 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
        return err;
 }

Quoted 3 lines · Vivek Goyal

If file is metacopy only, it is possible that lower is encrypted while other is not. In that case, report file as encrypted (despite the fact that only data is encrypted while metadata is not).

Better consult ext4 guys or find out which user tools care about this flag and what they could do in response to this flag.

When I commented that we need to see what do to about all these flags I just gave encrypted flag as an example.

On a hunch I would say that we need a mask of statx flags that are readonly attributes of the data itself and that mask should probably contain encrypted and compressed to begin with.

I don't see a reason for 2 separate patches and certainly not for 2 separate helpers.

Quoted 15 lines

Signed-off-by Vivek Goyal <[email protected]>

---
 fs/overlayfs/inode.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

fs/overlayfs/inode.c +0 -0

index 7ba19a97a8da..15713d4ac2dd 100644
@@ -66,6 +66,16 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
        return err;
 }
 

fs/overlayfs/inode.c int ovl_setattr(struct dentry *dentry, struct iattr *attr) · line 77

Quoted 9 lines

fs/overlayfs/inode.c +9 -0

+static void ovl_stat_set_encryption(struct kstat *ustat, struct kstat *lstat) {
+       if (!((lstat->attributes_mask & STATX_ATTR_ENCRYPTED) &&
+           (lstat->attributes & STATX_ATTR_ENCRYPTED)))
+               return;
+
+       ustat->attributes |= STATX_ATTR_ENCRYPTED;
+       ustat->attributes_mask |= STATX_ATTR_ENCRYPTED;
+}
+

This looks buggy. you set STATX_ATTR_ENCRYPTED even if lower doesn't have STATX_ATTR_ENCRYPTED in attributes nor in attributes_mask.

If file is metacopy only, it is possible that lower is compressed while upper is not. In that case, report file as compressed (despite the fact that only data is compressed while metadata is not).

Signed-off-by Vivek Goyal <[email protected]>

If a file is copied up metadata only and later when same file is opened for WRITE, then data copy up takes place. We copy up data, remove METACOPY xattr and then set the UPPERDATA flag in ovl_entry->flags. While all these operations happen with oi->lock held, read side of oi->flags is lockless. That is another thread on another cpu can check if UPPERDATA flag is set or not.

So this gives us an ordering requirement w.r.t UPPERDATA flag. That is, if another cpu sees UPPERDATA flag set, then it should be guaranteed that effects of data copy up and remove xattr operations are also visible.

For example.

CPU1 CPU2
ovl_d_real() acquire(oi->lock)

  ovl_copy_up_flags()                    ovl_copy_up_data()
   ovl_dentry_needs_data_copy_up()       vfs_removexattr()
     ovl_test_flag(OVL_UPPERDATA)        ovl_set_flag(OVK_UPPERDATA)
                                        release(oi->lock)

Say CPU2 is copying up data and in the end sets UPPERDATA flag. But if CPU1 perceives the effects of setting UPPERDATA flag but not effects of preceeding operations, that would be a problem.

Hence this patch introduces smp_wmb() on setting UPPERDATA flag operation and smp_rmb() on UPPERDATA flag test operation.

May be some other lock or barrier is already covering it. But I am not sure what that is and is it obvious enough that we will not break it in future.

So hence trying to be safe here and introducing barriers explicitly for UPPERDATA flag/bit.

Signed-off-by Vivek Goyal <[email protected]>

---
 fs/overlayfs/util.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

fs/overlayfs/util.c +1 -1

index f70eea77c5c7..81b307c20a8d 100644
@@ -267,13 +267,26 @@ bool ovl_has_upperdata(struct dentry *dentry) {
        if (!ovl_should_check_upperdata(dentry))
                return true;
 
-       return ovl_test_flag(OVL_UPPERDATA, d_inode(dentry));
+       if (!ovl_test_flag(OVL_UPPERDATA, d_inode(dentry)))

Quoted 8 lines

fs/overlayfs/util.c +1 -1

@@ -267,13 +267,26 @@ bool ovl_has_upperdata(struct dentry *dentry) {
        if (!ovl_should_check_upperdata(dentry))
                return true;
 
-       return ovl_test_flag(OVL_UPPERDATA, d_inode(dentry));
+       if (!ovl_test_flag(OVL_UPPERDATA, d_inode(dentry)))

fs/overlayfs/util.c bool ovl_has_upperdata(struct dentry *dentry) { · line 280

Quoted 10 lines

fs/overlayfs/util.c +8 -0

+               return false;
+       /*
+        * Pairs with smp_wmb() in ovl_copy_up_meta_inode_data(). Make sure
+        * if setting of OVL_UPPERDATA is visible, then effects of writes
+        * before that are visible too.
+        */
+       smp_rmb();
+       return true;
 }
 

I think it is not right to have smp_rmb() is the locked path where it is not needed
too costly.

We should probably have a different variant for ovl_already_copied_up_locked() and if OVL_UPPERDATA is set for all non-eligible cases, ovl_has_upperdata() can have the smp_rmb() and locked variant can just can check the flag directly.

All the bits are in patches before this. So it is time to enable the metadata only copy up feature.

Signed-off-by Vivek Goyal <[email protected]>