linux-kernel - Re: [Patch] tmpfs: add fallocate support

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1321379039.12374.11.camel@nimitz>
Date:	Tue, 15 Nov 2011 09:43:59 -0800
From:	Dave Hansen <dave@...ux.vnet.ibm.com>
To:	Cong Wang <amwang@...hat.com>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	Hugh Dickins <hughd@...gle.com>, linux-mm@...ck.org,
	Lennart Poettering <lennart@...ttering.net>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Subject: Re: [Patch] tmpfs: add fallocate support

On Tue, 2011-11-15 at 18:23 +0800, Cong Wang wrote:
> > +static long shmem_fallocate(struct file *file, int mode,
> > +			    loff_t offset, loff_t len)
> > +{
> > +	struct inode *inode = file->f_path.dentry->d_inode;
> > +	struct address_space *mapping = inode->i_mapping;
> > +	struct shmem_inode_info *info = SHMEM_I(inode);
> > +	struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
> > +	pgoff_t start = DIV_ROUND_UP(offset, PAGE_CACHE_SIZE);
> > +	pgoff_t end = DIV_ROUND_UP((offset + len), PAGE_CACHE_SIZE);
> > +	pgoff_t index = start;
> > +	gfp_t gfp = mapping_gfp_mask(mapping);
> > +	loff_t i_size = i_size_read(inode);
> > +	struct page *page = NULL;
> > +	int ret;
> > +
> > +	if ((offset + len)<= i_size)
> > +		return 0;

This seems to say that if the fallocate() call ends before the end of
the file we should ignore the call.  In other words, this can only be
used to extend the file, but could not be used to fill in the holes of a
sparse file.  

Is there a reason it was done that way?

> > +	if (!(mode&  FALLOC_FL_KEEP_SIZE)) {
> > +		ret = inode_newsize_ok(inode, (offset + len));
> > +		if (ret)
> > +			return ret;
> > +	}

inode_newsize_ok()'s comments say:

 * inode_newsize_ok must be called with i_mutex held.

But I don't see any trace of it.

> > +	if (start == end) {
> > +		if (!(mode&  FALLOC_FL_KEEP_SIZE))
> > +			i_size_write(inode, offset + len);
> > +		return 0;
> > +	}

There's a whitespace borkage like that 'mode&' all over this patch.
Probably needs a little love.

> +       if (shmem_acct_block(info->flags))
> +               return -ENOSPC;
> +
> +       if (sbinfo->max_blocks) {
> +               unsigned long blocks = (end - index) * BLOCKS_PER_PAGE;
> +               if (blocks + percpu_counter_sum(&sbinfo->used_blocks)
> +                               >= sbinfo->max_blocks) {
> +                       ret = -ENOSPC;
> +                       goto unacct;
> +               }
> +       }
...
> > +	while (index<  end) {
> > +		if (sbinfo->max_blocks)
> > +			percpu_counter_add(&sbinfo->used_blocks, BLOCKS_PER_PAGE);
> > +
> > +		page = shmem_alloc_page(gfp, info, index);
> > +		if (!page) {
> > +			ret = -ENOMEM;
> > +			goto decused;
> > +		}
> > +
> > +		SetPageSwapBacked(page);
> > +		__set_page_locked(page);
> > +		ret = mem_cgroup_cache_charge(page, current->mm,
> > +						gfp&  GFP_RECLAIM_MASK);
> > +		if (!ret)
> > +			ret = shmem_add_to_page_cache(page, mapping, index,
> > +						gfp, NULL);
> > +		if (ret)
> > +			goto unlock;
> > +		lru_cache_add_anon(page);
> > +
> > +		spin_lock(&info->lock);
> > +		info->alloced++;
> > +		inode->i_blocks += BLOCKS_PER_PAGE;
> > +		inode->i_ctime = inode->i_mtime = CURRENT_TIME;
> > +		shmem_recalc_inode(inode);
> > +		spin_unlock(&info->lock);
> > +
> > +		clear_highpage(page);
> > +		flush_dcache_page(page);
> > +		SetPageUptodate(page);
> > +		unlock_page(page);
> > +		page_cache_release(page);
> > +		cond_resched();
> > +		index++;
> > +		if (!(mode&  FALLOC_FL_KEEP_SIZE))
> > +			i_size_write(inode, index<<  PAGE_CACHE_SHIFT);
> > +	

This seems to have borrowed quite generously from shmem_getpage_gfp().
Seems like some code consolidation is in order before this level of
copy-n-paste.

-- Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/