linux-ext4 - Re: [PATCH] jbd: Remove j

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]

Message-ID: <20111227185100.GA30094@dhcp-172-17-9-228.mtv.corp.google.com>
Date:	Tue, 27 Dec 2011 10:51:00 -0800
From:	Joel Becker <jlbec@...lplan.org>
To:	Jan Kara <jack@...e.cz>
Cc:	linux-ext4@...r.kernel.org, akmp@...e.cz, rjw@...k.pl,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] jbd: Remove j_barrier mutex

On Thu, Dec 22, 2011 at 03:07:45PM +0100, Jan Kara wrote:
> j_barrier mutex is used for serializing different journal lock operations.  The
> problem with it is that e.g. FIFREEZE ioctl results in process leaving kernel
> with j_barrier mutex held which makes lockdep freak out. Also hibernation code
> wants to freeze filesystem but it cannot do so because it then cannot hibernate
> the system because of mutex being locked.
> 
> So we remove j_barrier mutex and use direct wait on j_barrier_count instead.
> Since locking journal is a rare operation we don't have to care about fairness
> or such things.
> 
> CC: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Jan Kara <jack@...e.cz>

Strikes me as pretty reasonable.

>  void journal_lock_updates(journal_t *journal)
>  {
>  	DEFINE_WAIT(wait);
>  
> +wait:
> +	/* Wait for previous locked operation to finish */
> +	wait_event(journal->j_wait_transaction_locked,
> +		   journal->j_barrier_count == 0);
> +
>  	spin_lock(&journal->j_state_lock);
> +	/*
> +	 * Check reliably under the lock whether we are the ones winning the race
> +	 * and locking the journal
> +	 */
> +	if (journal->j_barrier_count > 0) {
> +		spin_unlock(&journal->j_state_lock);
> +		goto wait;
> +	}

I suppose I'd prefer:

	do {
		wait_event(journal->j_wait_transaction_locked,
			   journal->j_barrier_count == 0);

		spin_lock(&journal->j_state_lock);
		if (journal->j_barrier_count == 0)
			break;
		spin_unlock(&journal->j_state_lock);
	} while (1);
  	++journal->j_barrier_count;

because I hate using goto for trivial loops, but that's a nitpick.

ACK.

Joel

-- 

You can use a screwdriver to screw in screws or to clean your ears,
however, the latter needs real skill, determination and a lack of fear
of injuring yourself.  It is much the same with JavaScript.
	- Chris Heilmann

			http://www.jlbec.org/
			jlbec@...lplan.org
--
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