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
| ||
|
Date: Tue, 23 Nov 2010 21:54:32 +1100 From: Nick Piggin <npiggin@...nel.dk> To: Boaz Harrosh <bharrosh@...asas.com> Cc: Nick Piggin <npiggin@...nel.dk>, linux-fsdevel@...r.kernel.org, Andrew Morton <akpm@...ux-foundation.org>, Al Viro <viro@...IV.linux.org.uk>, linux-ext4@...r.kernel.org, linux-btrfs@...r.kernel.org, Jan Kara <jack@...e.cz>, Eric Sandeen <sandeen@...hat.com>, Theodore Ts'o <tytso@....edu> Subject: Re: [patch] fs: fix deadlocks in writeback_if_idle On Tue, Nov 23, 2010 at 12:26:31PM +0200, Boaz Harrosh wrote: > > * > > * Invoke writeback_inodes_sb if no writeback is currently underway. > > * Returns 1 if writeback was started, 0 if not. > > + * > > + * Even if 1 is returned, writeback may not be started if memory allocation > > + * fails. This function makes no guarantees about anything. > > */ > > int writeback_inodes_sb_if_idle(struct super_block *sb) > > { > > if (!writeback_in_progress(sb->s_bdi)) { > > - down_read(&sb->s_umount); > > - writeback_inodes_sb(sb); > > - up_read(&sb->s_umount); > > + bdi_start_writeback(sb->s_bdi, get_nr_dirty_pages()); > > return 1; > > - } else > > - return 0; > > + } > > + return 0; > > } > > EXPORT_SYMBOL(writeback_inodes_sb_if_idle); > > > > @@ -1172,17 +1173,18 @@ EXPORT_SYMBOL(writeback_inodes_sb_if_idl > > * > > * Invoke writeback_inodes_sb if no writeback is currently underway. > > * Returns 1 if writeback was started, 0 if not. > > + * > > + * Even if 1 is returned, writeback may not be started if memory allocation > > + * fails. This function makes no guarantees about anything. > > */ > > int writeback_inodes_sb_nr_if_idle(struct super_block *sb, > > unsigned long nr) > > { > > if (!writeback_in_progress(sb->s_bdi)) { > > - down_read(&sb->s_umount); > > - writeback_inodes_sb_nr(sb, nr); > > - up_read(&sb->s_umount); > > + bdi_start_writeback(sb->s_bdi, nr); > > return 1; > > - } else > > - return 0; > > + } > > + return 0; > > } > > EXPORT_SYMBOL(writeback_inodes_sb_nr_if_idle); > > > > static inline int writeback_inodes_sb_if_idle(struct super_block *sb) > { > return writeback_inodes_sb_nr_if_idle(sb, get_nr_dirty_pages()); > } > > In writeback.h, No? I didn't care enough to move it :P I don't know if it matters. > But it has a single user so please just kill it. > > Also writeback_inodes_sb_nr_if_idle() has a single user. Combined with above, > two users. Why not open code it in the two sites. It should be much > clearer to understand what the magic is all about? The filesystem shouldn't be aware of the details (the "magic") of how to kick writeback, so I think the abstraction is right as is. Thanks, Nick -- 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
Powered by blists - more mailing lists