Re: [Ext4 Secure Delete 7/7v4] ext4/jbd2: Secure Delete: Secure delete journal blocks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/10/2011 04:41 PM, Jonathan Corbet wrote:
On Mon, 10 Oct 2011 16:35:24 -0700
Allison Henderson<achender@xxxxxxxxxxxxxxxxxx>  wrote:

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

On Fri,  7 Oct 2011 00:11:05 -0700
Allison Henderson<achender@xxxxxxxxxxxxxxxxxx>   wrote:

+	/* 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

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux