Messages in this thread | | | From | Joakim Tjernlund <> | Subject | Re: [PATCH] jffs2: remove fd from the f->dents list immediately. | Date | Fri, 16 Mar 2018 12:39:25 +0000 |
| |
On Fri, 2018-03-16 at 19:05 +0800, Yufen Yu wrote: > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > > commit 15953580e79b ("[JFFS2] Improve getdents vs. f_pos handling on NOR flash.") > is introduced to resolve 'rm -r', which cannot remove all files: > http://lists.infradead.org/pipermail/linux-mtd/2007-October/019658.html > > However, it can cause the following issues: > > 1. 'deletion' dirents is alway in the f->dents list, wasting memory > resource. For example: > There is a file named 'file1'. Then we rename it: > mv file1 file2; > mv file2 file3; > ... > mv file99999 file1000000 > > When CONFIG_JFFS2_SUMMARY is not set, file1~file1000000 > always in the f->dents list. > > 2. Since the list become longer and longer, more CPU time is used > to traverse it. > > After reverting the commit, we test 'rm -r', which can remove all > files, and all seems OK!
UHH, this is mine (and Davids work from 2007)! I cannot remember any details this long afterwards but I guess you cannot just revert that part as it triggers some other bug, David?
Jocke
> > Signed-off-by: Yufen Yu <yuyufen@huawei.com> > --- > fs/jffs2/write.c | 33 +++++++++++++++++---------------- > 1 file changed, 17 insertions(+), 16 deletions(-) > > diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c > index cda9a361368e..1deed35beb50 100644 > --- a/fs/jffs2/write.c > +++ b/fs/jffs2/write.c > @@ -598,31 +598,32 @@ int jffs2_do_unlink(struct jffs2_sb_info *c, struct jffs2_inode_info *dir_f, > jffs2_add_fd_to_list(c, fd, &dir_f->dents); > mutex_unlock(&dir_f->sem); > } else { > + struct jffs2_full_dirent **prev = &dir_f->dents; > uint32_t nhash = full_name_hash(NULL, name, namelen); > > - fd = dir_f->dents; > /* We don't actually want to reserve any space, but we do > want to be holding the alloc_sem when we write to flash */ > mutex_lock(&c->alloc_sem); > mutex_lock(&dir_f->sem); > > - for (fd = dir_f->dents; fd; fd = fd->next) { > - if (fd->nhash == nhash && > - !memcmp(fd->name, name, namelen) && > - !fd->name[namelen]) { > - > - jffs2_dbg(1, "Marking old dirent node (ino #%u) @%08x obsolete\n", > - fd->ino, ref_offset(fd->raw)); > - jffs2_mark_node_obsolete(c, fd->raw); > - /* We don't want to remove it from the list immediately, > - because that screws up getdents()/seek() semantics even > - more than they're screwed already. Turn it into a > - node-less deletion dirent instead -- a placeholder */ > - fd->raw = NULL; > - fd->ino = 0; > - break; > + while ((*prev) && (*prev)->nhash <= nhash) { > + if ((*prev)->nhash == nhash && > + !memcmp((*prev)->name, name, namelen) && > + !(*prev)->name[namelen]) { > + > + struct jffs2_full_dirent *this = *prev; > + > + jffs2_dbg(1, "Marking old dirent node (ino #%u) @%08x obsolete\n", > + this->ino, ref_offset(this->raw)); > + *prev = this->next; > + jffs2_mark_node_obsolete(c, this->raw); > + jffs2_free_full_dirent(this); > + > + break; > } > + prev = &((*prev)->next); > } > + > mutex_unlock(&dir_f->sem); > } > > -- > 2.13.6 > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
| |