On 10/10/2011 04:41 PM, Jonathan Corbet wrote:

On Mon, 10 Oct 2011 16:35:24 -0700
Allison Henderson[off-list ref]  wrote:

quoted

On 10/10/2011 12:47 PM, Jonathan Corbet wrote:

quoted

One quick question:

On Fri,  7 Oct 2011 00:11:05 -0700
Allison Henderson[off-list ref]   wrote:

quoted

+	/* Secure delete any blocks still in our range */
+	if (jbd2_pblk_count>   0)
+		err = ext4_secure_delete_pblks(journal->j_inode,
+			jbd2_pblk_start, jbd2_pblk_count);
+
+out:
+	spin_unlock(&journal->j_pair_lock);

ext4_secure_delete_pblks() appears to do its job synchronously - it has
calls to things like sync_dirty_buffer() and such.  How can you do that
while holding ->j_pair_lock?

Thanks,

jon

Hi Jon,

Well j_pair_lock is a lock I added to protect the new list of vfs
->  jbd2 block pairs. It is locked by the journal commit thread to
update the list when ever a journal block is modified. The above
code here is called by the same thread that performs a punch hole or
truncate operation, not the journal commit thread. So I'm not
immediately seeing why there would be any lock problems. Is there
another case I'm missing?

The problem is that ext4_secure_delete_pblks() can sleep, unless I've
misunderstood things very badly.  That's not something you want to do
while holding a spinlock...

jon

Oh I see the concern now.  Ok, I can put in a semaphore instead.  Thx 
for catching that.  :)

Allison