linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@linux.intel.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Matthew Wilcox <matthew.r.wilcox@intel.com>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v11 02/21] block: Change direct_access calling convention
Date: Thu, 16 Oct 2014 15:39:21 -0400	[thread overview]
Message-ID: <20141016193921.GC11522@wil.cx> (raw)
In-Reply-To: <20141016084550.GA19075@thinkos.etherlink>

On Thu, Oct 16, 2014 at 10:45:50AM +0200, Mathieu Desnoyers wrote:
> > -static int
> > +static long
> >  axon_ram_direct_access(struct block_device *device, sector_t sector,
> > -		       void **kaddr, unsigned long *pfn)
> > +		       void **kaddr, unsigned long *pfn, long size)
> 
> Why "long" as type for size ? What is the intent to have it signed, and
> why using a 32-bit type on 32-bit architectures rather than 64-bit ?
> Can we run into issues if we try to map a >2GB file on 32-bit
> architectures ?

The interface requires that the entirety of the pmem be mapped at
all times (see the void **kaddr).  So the total amount of pmem in the
system can't be larger than 4GB on a 32-bit system.  On x86-32, that's
actually limited to 1GB (because we give userspace 3GB), so the problem
doesn't come up.  Maybe this would be more of a potetial problem on
other architectures.

As noted elsewhere in the thread, it would be possible, and maybe
desirable, to remove the need to have all of pmem mapped into the kernel
address space at all times, but I'm not looking to solve that problem
with this patch-set.

The intent of having it signed is that users pass in the size they want
to have and are returned the size they actually got.  Since the function
must be able to return an error, keeping size signed is natural.

> > +long bdev_direct_access(struct block_device *bdev, sector_t sector,
> > +			void **addr, unsigned long *pfn, long size)
> > +{
> > +	long avail;
> > +	const struct block_device_operations *ops = bdev->bd_disk->fops;
> > +
> > +	if (size < 0)
> > +		return size;
> 
> I'm wondering how we should handle size == 0 here. Should it be accepted
> or refused ?

It is a bit of a bizarre case.  I'm inclined to the current behaviour
of saying "this is the address where you can access zero bytes" :-)

But maybe it indicates a bug in the caller, and being noisy about it
would result in the caller getting fixed.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2014-10-16 19:39 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-25 20:33 [PATCH v11 00/21] Add support for NV-DIMMs to ext4 Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 01/21] axonram: Fix bug in direct_access Matthew Wilcox
2014-10-16  7:52   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 02/21] block: Change direct_access calling convention Matthew Wilcox
2014-10-16  8:45   ` Mathieu Desnoyers
2014-10-16 19:39     ` Matthew Wilcox [this message]
2014-09-25 20:33 ` [PATCH v11 03/21] mm: Fix XIP fault vs truncate race Matthew Wilcox
2014-10-16  8:56   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 04/21] mm: Allow page fault handlers to perform the COW Matthew Wilcox
2014-10-16  9:12   ` Mathieu Desnoyers
2014-10-16 19:48     ` Matthew Wilcox
2014-10-17 15:35       ` Mathieu Desnoyers
2014-10-18 17:22         ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 05/21] vfs,ext2: Introduce IS_DAX(inode) Matthew Wilcox
2014-10-16  9:35   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 06/21] vfs: Add copy_to_iter(), copy_from_iter() and iov_iter_zero() Matthew Wilcox
2014-10-16 13:33   ` Mathieu Desnoyers
2014-10-16 13:59     ` Matthew Wilcox
2014-10-16 14:12       ` Mathieu Desnoyers
2014-10-16 22:21         ` Matthew Wilcox
2014-10-17 15:39           ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 07/21] dax,ext2: Replace XIP read and write with DAX I/O Matthew Wilcox
2014-10-16  9:50   ` Mathieu Desnoyers
2014-10-16 19:51     ` Matthew Wilcox
2014-10-16 22:33       ` Matthew Wilcox
2014-10-17 15:52         ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 08/21] dax,ext2: Replace ext2_clear_xip_target with dax_clear_blocks Matthew Wilcox
2014-10-16 10:05   ` Mathieu Desnoyers
2014-10-16 21:22     ` Matthew Wilcox
2014-10-17 15:45       ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 09/21] dax,ext2: Replace the XIP page fault handler with the DAX page fault handler Matthew Wilcox
2014-10-16 10:20   ` Mathieu Desnoyers
2014-10-16 21:29     ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 10/21] dax,ext2: Replace xip_truncate_page with dax_truncate_page Matthew Wilcox
2014-10-16 10:28   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 11/21] dax: Replace XIP documentation with DAX documentation Matthew Wilcox
2014-10-16 12:08   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 12/21] vfs: Remove get_xip_mem Matthew Wilcox
2014-10-16 12:14   ` Mathieu Desnoyers
2014-10-16 21:44     ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 13/21] ext2: Remove ext2_xip_verify_sb() Matthew Wilcox
2014-10-16 12:18   ` Mathieu Desnoyers
2014-10-16 21:45     ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 14/21] ext2: Remove ext2_use_xip Matthew Wilcox
2014-10-16 12:20   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 15/21] ext2: Remove xip.c and xip.h Matthew Wilcox
2014-10-16 12:21   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 16/21] vfs,ext2: Remove CONFIG_EXT2_FS_XIP and rename CONFIG_FS_XIP to CONFIG_FS_DAX Matthew Wilcox
2014-10-16 12:26   ` Mathieu Desnoyers
2014-10-16 21:52     ` Matthew Wilcox
2014-09-25 20:33 ` [PATCH v11 17/21] ext2: Remove ext2_aops_xip Matthew Wilcox
2014-10-16 12:29   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 18/21] ext2: Get rid of most mentions of XIP in ext2 Matthew Wilcox
2014-10-16 12:32   ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 19/21] dax: Add dax_zero_page_range Matthew Wilcox
2014-10-16 12:38   ` Mathieu Desnoyers
2014-10-16 22:01     ` Matthew Wilcox
2014-10-17 15:49       ` Mathieu Desnoyers
2014-10-18 17:41         ` Matthew Wilcox
2014-10-18 21:16           ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 20/21] ext4: Add DAX functionality Matthew Wilcox
2014-10-16 12:56   ` Mathieu Desnoyers
2014-10-16 22:16     ` Matthew Wilcox
2014-10-17 15:42       ` Mathieu Desnoyers
2014-09-25 20:33 ` [PATCH v11 21/21] brd: Rename XIP to DAX Matthew Wilcox
2014-10-16 13:00   ` Mathieu Desnoyers
2015-03-24 18:50   ` Matt Mullins
2015-03-25  3:25     ` Dave Chinner
2015-03-26 17:09     ` Should implementations of ->direct_access be allowed to sleep? Matthew Wilcox
2015-03-26 19:32       ` Dave Chinner
2015-03-29  8:02         ` Boaz Harrosh
2015-03-29  9:13           ` Boaz Harrosh
2014-09-25 20:47 ` [PATCH v11 00/21] Add support for NV-DIMMs to ext4 Matthew Wilcox
2014-09-30  9:45 ` Valdis.Kletnieks
2014-09-30 14:48   ` Matthew Wilcox
2014-09-30 14:53     ` Valdis.Kletnieks
2014-09-30 16:08       ` Matthew Wilcox
2014-09-30 17:10         ` Zuckerman, Boris
2014-09-30 19:24           ` Matthew Wilcox
2014-09-30 19:31             ` Zuckerman, Boris
2014-09-30 20:37         ` Valdis.Kletnieks
2014-09-30 21:25           ` Andreas Dilger
2014-09-30 21:52             ` Valdis.Kletnieks
2014-10-01 15:45               ` Jeff Moyer
2014-10-01 17:10                 ` Valdis.Kletnieks
2014-10-01 17:17                 ` Valdis.Kletnieks
2014-10-16  7:39 ` Mathieu Desnoyers
2014-10-16 14:11   ` Matthew Wilcox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141016193921.GC11522@wil.cx \
    --to=willy@linux.intel.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=matthew.r.wilcox@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).