Messages in this thread | | | Subject | Re: Possible UDF locking error? | From | Steve Magnani <> | Date | Mon, 25 Mar 2019 13:23:57 -0500 |
| |
On 3/25/19 11:42 AM, Jan Kara wrote: > Hi! > > On Sat 23-03-19 15:14:05, Steve Magnani wrote: >> ... >> >> In contrast, udf_setup_indirect_aext(), which constructs an AED, >> has this sequence: >> >> bh = udf_tgetblk(...); // calls sb_getblk() >> lock_buffer(bh); >> memset(bh->b_data, 0, inode->i_sb->s_blocksize); >> >> set_buffer_uptodate(bh); >> unlock_buffer(bh); >> mark_buffer_dirty_inode(bh); >> >> // <snip>other code to populate AED data in the block</snip> >> >> In this case the population of the block occurs without >> the protection of the lock. >> >> Because the block has been marked dirty, does this mean that >> writeback could occur at any point during population? > Yes. Thanks for noticing this! > >> There is one path through udf_setup_indirect_aext() where >> mark_buffer_dirty_inode() gets called again after population is >> complete, which I suppose could heal a partial writeout, but there is >> also another path in which the buffer does not get marked dirty again. > Generally, we add new extents to the created indirect extent which dirties > the buffer and that should fix the problem. But you are definitely right > that the code is suspicious and should be fixed. Will you send a patch? > > Honza
Sure. There's at least one other place where it looked like there might be a similar issue.
Steve
| |