Re: [PATCH 6/6] dax: Avoid page invalidation races and unnecessary radix tree traversals

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

 



On Mon, Oct 03, 2016 at 04:58:02PM +0200, Jan Kara wrote:
> On Wed 28-09-16 10:20:34, Dave Chinner wrote:
> > On Tue, Sep 27, 2016 at 06:43:35PM +0200, Jan Kara wrote:
> > > @@ -1410,6 +1422,17 @@ iomap_dax_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
> > >  	if (WARN_ON_ONCE(iomap->type != IOMAP_MAPPED))
> > >  		return -EIO;
> > >  
> > > +	/*
> > > +	 * Write can allocate block for an area which has a hole page mapped
> > > +	 * into page tables. We have to tear down these mappings so that data
> > > +	 * written by write(2) is visible in mmap.
> > > +	 */
> > > +	if (iomap->flags & IOMAP_F_NEW && inode->i_mapping->nrpages) {
> > 
> > gcc should be throwing warnings about that:
> > 
> > 	if ((iomap->flags & IOMAP_F_NEW) && inode->i_mapping->nrpages) {
> 
> Actually the bitwise '&' takes precedense over the logical '&&' so the
> evaluation order ends up being correct.

Yes, I know that.

However, my concern is that such expressions don't indicate the
/intent/ of the author and so it can be difficult when reading the
code to determine if the logic is correct or whether it is a typo
and is wrong. In many cases, & and && will function identically for
the tested cases, so neither cursory review or testing would catch
something like this being wrong.

> But I agree it's better to be
> explicit with parenthesis here. Fixed.

Yup, a little bit of "documentation" goes a long way :P

Cheers,

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



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux