On Sat, 24 Jun 2006, Peter Zijlstra wrote:
> > > + if ((pgprot_val(vma->vm_page_prot) == pgprot_val(vm_page_prot) &&
> > > + ((vm_flags & (VM_WRITE|VM_SHARED|VM_PFNMAP|VM_INSERTPAGE)) ==
> > > + (VM_WRITE|VM_SHARED)) &&
> > > + vma->vm_file && vma->vm_file->f_mapping &&
> > > + mapping_cap_account_dirty(vma->vm_file->f_mapping)) ||
> > > + (vma->vm_ops && vma->vm_ops->page_mkwrite))
> > > + vma->vm_page_prot =
> > > + protection_map[vm_flags & (VM_READ|VM_WRITE|VM_EXEC)];
> > > +
> >
> > I'm dazzled by the beauty of it!
>
> It's a real beauty isn't it :-)
Since Hugh pointed that out..
It really would be nice to just encapsulate that as an inline function of
its own, and move the comment at the top of it to be at the top of the
inline function.
Just make it something like
/*
* Some shared mappigns will want the pages marked read-only
* to track write events. If so, we'll downgrade vm_page_prot
* to the private version (using protection_map[] without the
* VM_SHARED bit).
*/
static inline int vma_wants_writenotify(struct vm_area_struct *vma)
{
unsigned int vm_flags = vma->vm_flags;
/* If it was private or non-writable, the write bit is already clear */
if ((vm_flags & (VM_SHARED | VM_WRITE)) != ((VM_SHARED | VM_WRITE))
return 0;
/* The open routine did something to the protections already? */
if (pgprot_val(vma->vm_page_prot) !=
pgprot_val(protection_map[vm_flags & (VM_SHARED|VM_READ|VM_WRITE|VM_EXEC)]))
return 0;
/* The backer wishes to know when pages are first written to? */
if (vma->vm_ops && vma->vm_ops->page_mkwrite)
return 1;
/* Specialty mapping? */
if (vm_flags & (VM_PFNMAP|VM_INSERTPAGE))
return 0;
/* Can the mapping track the dirty pages? */
return vma->vm_file && vma->vm_file->f_mapping &&
mapping_cap_account_dirty(vma->vm_file->f_mapping);
}
(And no, I didn't make sure to test that it gives the same answer as your
version, it's just a more readable version of what I think your version
tests ;)
And then just use it with
if (vma_wants_writenotify(vma))
vma->vm_page_prot = protection_map[vm_flags & (VM_READ|VM_WRITE|VM_EXEC)];
which would appear to be more readable.
Yeah, the compiler may do worse. Or it may not. It usually pays to try to
write code more readably, sometimes the compiler ends up understanding it
better too ;)
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
