linux-ext4 - Re: [PATCH RFC 4/5] ext2fs: parallel bitmap loading

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: <20201211223512.GC575698@mit.edu>
Date:   Fri, 11 Dec 2020 17:35:12 -0500
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Saranya Muruganandam <saranyamohan@...gle.com>,
        Wang Shilong <wshilong@....com>
Subject: Re: [PATCH RFC 4/5] ext2fs: parallel bitmap loading

On Thu, Dec 10, 2020 at 05:12:09PM -0700, Andreas Dilger wrote:
> > @@ -329,12 +369,20 @@ static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
> > 				}
> > 				if (!bitmap_tail_verify((unsigned char *) block_bitmap,
> > 							block_nbytes, fs->blocksize - 1))
> > -					tail_flags |= EXT2_FLAG_BBITMAP_TAIL_PROBLEM;
> > +					*tail_flags |= EXT2_FLAG_BBITMAP_TAIL_PROBLEM;
> > 			} else
> > 				memset(block_bitmap, 0, block_nbytes);
> > 			cnt = block_nbytes << 3;
> > +#ifdef HAVE_PTHREAD
> > +			if (mutex)
> > +				pthread_mutex_lock(mutex);
> > +#endif
> > 			retval = ext2fs_set_block_bitmap_range2(fs->block_map,
> > 					       blk_itr, cnt, block_bitmap);
> > +#ifdef HAVE_PTHREAD
> > +			if (mutex)
> > +				pthread_mutex_unlock(mutex);
> > +#endif
> 
> (style) It wouldn't be terrible to have wrappers around these functions
> instead of inline #ifdef in the few places they are used, like:
> 
> #ifdef HAVE_PTHREAD
> static void unix_pthread_mutex_lock(pthread_mutex_t *mutex)
> {
> 	if (mutex)
> 		pthread_mutex_lock(mutex);
> }
> static void unix_pthread_mutex_unlock(pthread_mutex_t *mutex)
> {
> 	if (mutex)
> 		pthread_mutex_unlock(mutex);
> }
> #else
> #define unix_pthread_mutex_lock(mutex) do {} while (0)
> #define unix_pthread_mutex_unlock(mutex) do {} while (0)
> #endif

We'd also need to have a typedef for mutex_t which is either
pthreads_mutex_t if pthreads are available, or an int (or some other
placeholder type) if it isn't.

I had tried to make sure that rw_bitmaps.c will correctly compile with
HAVE_PTHREAD and HAVE_PTHREAD_H are undefined.  It looks like I didn't
quite get it completely working since there's at leasts one function
signature where we have an unprotected use of pthread_mutex_t, so
that's something we should check before finalizing the patch --- in
addition to the unprotected use of pthread_mutex_{lock,unlock} that
you pointed out.

      		    	   	 		   - Ted