On Sat, Dec 15, 2018 at 2:23 PM Hou Tao <hout...@huawei.com> wrote:
>
> ping ?
Sorry for the delay.
I'll try now to review all the pending jffs2 patches.
> On 2018/12/9 14:21, Hou Tao wrote:
> > For xattr modification, we do not write a new jffs2_raw_xref with
> > delete marker into flash, so if a xattr is modified then removed,
> > and the old xref & xdatum are not erased by GC, after reboot or
> > remount, the new xattr xref will be dead but the old xattr xref
> > will be alive, and we will get the overwritten xattr instead of
> > non-existent error when reading the removed xattr.
> >
> > Fix it by writing the deletion mark for xattr overwrite.
> >
> > Fixes: 8a13695cbe4e ("[JFFS2][XATTR] rid unnecessary writing of delete
> > marker.")
> > Signed-off-by: Hou Tao <hout...@huawei.com>
> > ---
> > fs/jffs2/xattr.c | 55
> > +++++++++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 49 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
> > index da3e18503c65..b2d6072f34af 100644
> > --- a/fs/jffs2/xattr.c
> > +++ b/fs/jffs2/xattr.c
> > @@ -573,6 +573,15 @@ static struct jffs2_xattr_ref *create_xattr_ref(struct
> > jffs2_sb_info *c, struct
> > return ref; /* success */
> > }
> >
> > +static void move_xattr_ref_to_dead_list(struct jffs2_sb_info *c,
> > + struct jffs2_xattr_ref *ref)
> > +{
> > + spin_lock(&c->erase_completion_lock);
> > + ref->next = c->xref_dead_list;
> > + c->xref_dead_list = ref;
> > + spin_unlock(&c->erase_completion_lock);
> > +}
> > +
> > static void delete_xattr_ref(struct jffs2_sb_info *c, struct
> > jffs2_xattr_ref *ref)
> > {
> > /* must be called under down_write(xattr_sem) */
> > @@ -582,10 +591,7 @@ static void delete_xattr_ref(struct jffs2_sb_info *c,
> > struct jffs2_xattr_ref *re
> > ref->xseqno |= XREF_DELETE_MARKER;
> > ref->ino = ref->ic->ino;
> > ref->xid = ref->xd->xid;
> > - spin_lock(&c->erase_completion_lock);
> > - ref->next = c->xref_dead_list;
> > - c->xref_dead_list = ref;
> > - spin_unlock(&c->erase_completion_lock);
> > + move_xattr_ref_to_dead_list(c, ref);
> >
> > dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
> > ref->ino, ref->xid, ref->xseqno);
> > @@ -1090,6 +1096,40 @@ int do_jffs2_getxattr(struct inode *inode, int
> > xprefix, const char *xname,
> > return rc;
> > }
> >
> > +static void do_jffs2_delete_xattr_ref(struct jffs2_sb_info *c,
> > + struct jffs2_xattr_ref *ref)
> > +{
> > + uint32_t request, length;
> > + int err;
> > + struct jffs2_xattr_datum *xd;
> > +
> > + request = PAD(sizeof(struct jffs2_raw_xref));
> > + err = jffs2_reserve_space(c, request, &length,
> > + ALLOC_NORMAL, JFFS2_SUMMARY_XREF_SIZE);
> > + down_write(&c->xattr_sem);
> > + if (err) {
> > + JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n",
> > + err, request);
> > + delete_xattr_ref(c, ref);
> > + up_write(&c->xattr_sem);
> > + return;
> > + }
> > +
> > + xd = ref->xd;
> > + ref->ino = ref->ic->ino;
> > + ref->xid = xd->xid;
> > + ref->xseqno |= XREF_DELETE_MARKER;
> > + save_xattr_ref(c, ref);
> > +
> > + move_xattr_ref_to_dead_list(c, ref);
> > + dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
> > + ref->ino, ref->xid, ref->xseqno);
> > + unrefer_xattr_datum(c, xd);
> > +
> > + up_write(&c->xattr_sem);
> > + jffs2_complete_reservation(c);
> > +}
> > +
> > int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
> > const char *buffer, size_t size, int flags)
> > {
> > @@ -1097,7 +1137,7 @@ int do_jffs2_setxattr(struct inode *inode, int
> > xprefix, const char *xname,
> > struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
> > struct jffs2_inode_cache *ic = f->inocache;
> > struct jffs2_xattr_datum *xd;
> > - struct jffs2_xattr_ref *ref, *newref, **pref;
> > + struct jffs2_xattr_ref *ref, *newref, *oldref, **pref;
> > uint32_t length, request;
> > int rc;
> >
> > @@ -1113,6 +1153,7 @@ int do_jffs2_setxattr(struct inode *inode, int
> > xprefix, const char *xname,
> > return rc;
> > }
> >
> > + oldref = NULL;
> > /* Find existing xattr */
> > down_write(&c->xattr_sem);
> > retry:
> > @@ -1196,11 +1237,13 @@ int do_jffs2_setxattr(struct inode *inode, int
> > xprefix, const char *xname,
> > rc = PTR_ERR(newref);
> > unrefer_xattr_datum(c, xd);
> > } else if (ref) {
> > - delete_xattr_ref(c, ref);
> > + oldref = ref;
> > }
> > out:
> > up_write(&c->xattr_sem);
> > jffs2_complete_reservation(c);
> > + if (oldref)
> > + do_jffs2_delete_xattr_ref(c, oldref);
> > return rc;
> > }
> >
> >
>
--
Thanks,
//richard
