linux-ext4 - Re: [PATCH 2/2] ext4: Reduce contention on s_orphan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 20 May 2014 23:03:54 +0200
From:	Jan Kara <jack@...e.cz>
To:	Thavatchai Makphaibulchoke <thavatchai.makpahibulchoke@...com>
Cc:	Jan Kara <jack@...e.cz>, Ted Tso <tytso@....edu>,
	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 2/2] ext4: Reduce contention on s_orphan_lock

On Tue 20-05-14 10:45:07, Thavatchai Makphaibulchoke wrote:
> On 05/20/2014 06:45 AM, Jan Kara wrote:
> > +	if (dirty) {
> > +		err = ext4_handle_dirty_super(handle, sb);
> > +		rc = ext4_mark_iloc_dirty(handle, inode, &iloc);
> > +		if (!err)
> > +			err = rc;
> > +		if (err) {
> > +			/*
> > +			 * We have to remove inode from in-memory list if
> > + 			 * addition to on disk orphan list failed. Stray orphan
> > +			 * list entries can cause panics at unmount time.
> > +			 */
> > +			mutex_lock(&sbi->s_orphan_lock);
> > +			list_del(&EXT4_I(inode)->i_orphan);
> > +			mutex_unlock(&sbi->s_orphan_lock);
> > +		}
> > +	}
> 
> Sorry Jan, I just noticed this.
> 
> I don't believe you could this optimization either.  Since you drop the
> s_oprhan_lock in between, you essentially have an interval where there is
> a stray in-memory orphan and could cause a panic as the comment above
> mentioned.
  No, I think we are fine in this case. I'm not sure what race you are
exactly thinking of but unmount cannot certainly happen since we have a
reference to active inode in the filesystem.
 
> As for comments regarding ext4_mark_iloc() optimization, in your case
> since you are holding the i_mutex, should not that prevent the inode from
> being reclaimed?
  Yes, that prevents the inode from being reclaimed but it doesn't prevent
the previous inode in the list from being reclaimed and we need to update
also that one...

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html