* Re: [RFC v3 1/1] Input: Add userio module
From: Dmitry Torokhov @ 2015-07-22 21:39 UTC (permalink / raw)
To: Stephen Chandler Paul
Cc: Andrew Morton, Mauro Carvalho Chehab, Greg KH, Arnd Bergmann,
Joe Perches, Jiri Slaby, Vishnu Patekar, Sebastian Ott, linux-doc,
linux-kernel, linux-input, linux-api, Benjamin Tissoires,
Hans de Goede
In-Reply-To: <1437594773-22274-2-git-send-email-cpaul@redhat.com>
Hi Stephen,
On Wed, Jul 22, 2015 at 03:52:53PM -0400, Stephen Chandler Paul wrote:
> +static int userio_char_open(struct inode *inode, struct file *file)
> +{
> + struct userio_device *userio;
> +
> + userio = devm_kzalloc(userio_misc.this_device,
> + sizeof(struct userio_device), GFP_KERNEL);
> + if (!userio)
> + return -ENOMEM;
Never ever use devm_ API outside of device probe() calls because
devm-managed resources only freed after failed probe() or after remove()
callback on driver model device and not any other device (block, char,
etc) in the system.
> +
> + spin_lock_init(&userio->head_lock);
> + mutex_init(&userio->tail_lock);
> + init_waitqueue_head(&userio->waitq);
> +
> + userio->serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
> + if (!userio->serio) {
> + devm_kfree(userio_misc.this_device, userio);
And indeed here you have ot manually release the acquired resource. So
the only benefit you got form devm* is wasted resousrces.
> + return -ENOMEM;
> + }
> +
> + userio->serio->write = userio_device_write;
> + userio->serio->port_data = userio;
> +
> + file->private_data = userio;
> +
> + return 0;
> +}
> +
> +static int userio_char_release(struct inode *inode, struct file *file)
> +{
> + struct userio_device *userio = file->private_data;
> +
> + if (userio->serio) {
> + userio->serio->port_data = NULL;
No need to reset, it is going away.
> +
> + if (userio->running)
> + serio_unregister_port(userio->serio);
> + else
> + kfree(userio->serio);
> + }
> +
> + devm_kfree(userio_misc.this_device, userio);
> +
> + return 0;
> +}
> +
> +static ssize_t userio_char_read(struct file *file, char __user *buffer,
> + size_t count, loff_t *ppos)
> +{
> + struct userio_device *userio = file->private_data;
> + int ret;
> + size_t nonwrap_len, copylen;
> +
> + if (!count)
> + return 0;
> +
> + if (file->f_flags & O_NONBLOCK && userio->head == userio->tail)
> + return -EAGAIN;
> + else {
This is racy. If there was data and other thread "stole" it here then
your O_NONBLOCK will block. See evdev_read() how you supposed to handle
this.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Andrew Morton @ 2015-07-22 22:03 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm, linux-kernel, linux-api, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Davidlohr Bueso, Aneesh Kumar,
Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
On Tue, 21 Jul 2015 11:09:34 -0700 Mike Kravetz <mike.kravetz@oracle.com> wrote:
> Changes in this revision address the minor comment and function name
> issues brought up by Naoya Horiguchi. Patch set is also rebased on
> current "mmotm/since-4.1". This revision does not introduce any
> functional changes.
>
> As suggested during the RFC process, tests have been proposed to
> libhugetlbfs as described at:
> http://librelist.com/browser//libhugetlbfs/2015/6/25/patch-tests-add-tests-for-fallocate-system-call/
> fallocate(2) man page modifications are also necessary to specify
> that fallocate for hugetlbfs only operates on whole pages. This
> change will be submitted once the code has stabilized and been
> proposed for merging.
>
> hugetlbfs is used today by applications that want a high degree of
> control over huge page usage. Often, large hugetlbfs files are used
> to map a large number huge pages into the application processes.
> The applications know when page ranges within these large files will
> no longer be used, and ideally would like to release them back to
> the subpool or global pools for other uses. The fallocate() system
> call provides an interface for preallocation and hole punching within
> files. This patch set adds fallocate functionality to hugetlbfs.
>
> v4:
> Renamed vma_abort_reservation() to vma_end_reservation().
>
> ...
>
> v3 patch series:
> Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>
eek. Please don't hide the reviews and acks in [0/n] like this. I
don't expect to see them there, which causes me to park the patch
series until I'm feeling awake enough to undertake the patchset's first
ever review.
--
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>
^ permalink raw reply
* Re: [PATCH v4 01/10] mm/hugetlb: add cache of descriptors to resv_map for region_add
From: Andrew Morton @ 2015-07-22 22:03 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm, linux-kernel, linux-api, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Davidlohr Bueso, Aneesh Kumar,
Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <1437502184-14269-2-git-send-email-mike.kravetz@oracle.com>
On Tue, 21 Jul 2015 11:09:35 -0700 Mike Kravetz <mike.kravetz@oracle.com> wrote:
> fallocate hole punch will want to remove a specific range of
> pages. When pages are removed, their associated entries in
> the region/reserve map will also be removed. This will break
> an assumption in the region_chg/region_add calling sequence.
> If a new region descriptor must be allocated, it is done as
> part of the region_chg processing. In this way, region_add
> can not fail because it does not need to attempt an allocation.
>
> To prepare for fallocate hole punch, create a "cache" of
> descriptors that can be used by region_add if necessary.
> region_chg will ensure there are sufficient entries in the
> cache. It will be necessary to track the number of in progress
> add operations to know a sufficient number of descriptors
> reside in the cache. A new routine region_abort is added to
> adjust this in progress count when add operations are aborted.
> vma_abort_reservation is also added for callers creating
> reservations with vma_needs_reservation/vma_commit_reservation.
>
> ...
>
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -35,6 +35,9 @@ struct resv_map {
> struct kref refs;
> spinlock_t lock;
> struct list_head regions;
> + long adds_in_progress;
> + struct list_head rgn_cache;
> + long rgn_cache_count;
> };
Linux style is to spell words out fully: rgn->region. One advantage of
doing this is that we get consistency: when there's no doubt about how
a thing is spelled we don't end up with various different terms for the
same thing. Note that we already have resv_map.regions and struct
file_region.
To avoid a respin I think I'll just do a s/rgn/region/g on the patches
then take a look for 80 col overflow - shout at me if you dislike that.
And yes, resv_map should have been called reservation_map. I suck.
And there's region_chg. Who wrote this stuff.
As an off-topic cleanup, resv_map and its associated stuff could be
moved out of include/linux/hugetlb.h and made private to hugetlb.c with
some minor changes.
Finally, the data structure definition site is a great place at which
to document the overall design. What the fields do, how they interact,
locking design, etc. eg, what data structure is at
resv_map.region_cache, how is it managed, what purpose does it serve
etc.
And adds_in_progress is interesting. I see the comment over
region_abort() (rgn_abort?!) but haven't quite soaked that in yet.
>
> ...
>
> @@ -312,11 +339,16 @@ static long region_add(struct resv_map *resv, long f, long t)
> * so that the subsequent region_add call will have all the
> * regions it needs and will not fail.
> *
> + * Upon entry, region_chg will also examine the cache of
> + * region descriptors associated with the map. If there
"are"
> + * not enough descriptors cached, one will be allocated
> + * for the in progress add operation.
> + *
> * Returns the number of huge pages that need to be added
> * to the existing reservation map for the range [f, t).
> * This number is greater or equal to zero. -ENOMEM is
> - * returned if a new file_region structure is needed and can
> - * not be allocated.
> + * returned if a new file_region structure or cache entry
> + * is needed and can not be allocated.
> */
> static long region_chg(struct resv_map *resv, long f, long t)
> {
>
> ...
>
--
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>
^ permalink raw reply
* Re: [PATCH v4 03/10] mm/hugetlb: expose hugetlb fault mutex for use by fallocate
From: Andrew Morton @ 2015-07-22 22:03 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm, linux-kernel, linux-api, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Davidlohr Bueso, Aneesh Kumar,
Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <1437502184-14269-4-git-send-email-mike.kravetz@oracle.com>
On Tue, 21 Jul 2015 11:09:37 -0700 Mike Kravetz <mike.kravetz@oracle.com> wrote:
> Minor name changes to
> be more consistent with other global hugetlb symbols.
>
> ...
>
> - kfree(htlb_fault_mutex_table);
> + kfree(hugetlb_fault_mutex_table);
yay ;)
--
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>
^ permalink raw reply
* Re: [PATCH v4 09/10] hugetlbfs: add hugetlbfs_fallocate()
From: Andrew Morton @ 2015-07-22 22:03 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Davidlohr Bueso, Aneesh Kumar,
Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <1437502184-14269-10-git-send-email-mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On Tue, 21 Jul 2015 11:09:43 -0700 Mike Kravetz <mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> This is based on the shmem version, but it has diverged quite
> a bit. We have no swap to worry about, nor the new file sealing.
> Add synchronication via the fault mutex table to coordinate
> page faults, fallocate allocation and fallocate hole punch.
>
> What this allows us to do is move physical memory in and out of
> a hugetlbfs file without having it mapped. This also gives us
> the ability to support MADV_REMOVE since it is currently
> implemented using fallocate(). MADV_REMOVE lets madvise() remove
> pages from the middle of a hugetlbfs file, which wasn't possible
> before.
>
> hugetlbfs fallocate only operates on whole huge pages.
>
> ...
>
> +static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
> + loff_t len)
> +{
> + struct inode *inode = file_inode(file);
> + struct address_space *mapping = inode->i_mapping;
> + struct hstate *h = hstate_inode(inode);
> + struct vm_area_struct pseudo_vma;
> + struct mm_struct *mm = current->mm;
> + loff_t hpage_size = huge_page_size(h);
> + unsigned long hpage_shift = huge_page_shift(h);
> + pgoff_t start, index, end;
> + int error;
> + u32 hash;
> +
> + if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
> + return -EOPNOTSUPP;
EOPNOTSUPP is a networking thing. It's inappropriate here.
The problem is that if this error is ever returned to userspace, the
user will be sitting looking at "Operation not supported on transport
endpoint" and wondering what went wrong in the networking stack.
> + if (mode & FALLOC_FL_PUNCH_HOLE)
> + return hugetlbfs_punch_hole(inode, offset, len);
> +
> + /*
> + * Default preallocate case.
> + * For this range, start is rounded down and end is rounded up
> + * as well as being converted to page offsets.
> + */
> + start = offset >> hpage_shift;
> + end = (offset + len + hpage_size - 1) >> hpage_shift;
> +
> + mutex_lock(&inode->i_mutex);
> +
> + /* We need to check rlimit even when FALLOC_FL_KEEP_SIZE */
> + error = inode_newsize_ok(inode, offset + len);
> + if (error)
> + goto out;
> +
> + /*
> + * Initialize a pseudo vma that just contains the policy used
> + * when allocating the huge pages. The actual policy field
> + * (vm_policy) is determined based on the index in the loop below.
> + */
> + memset(&pseudo_vma, 0, sizeof(struct vm_area_struct));
> + pseudo_vma.vm_flags = (VM_HUGETLB | VM_MAYSHARE | VM_SHARED);
> + pseudo_vma.vm_file = file;
triviata: we could have just done
struct vm_area_struct pseudo_vma = {
.vm_flags = ...
.vm_file = file;
};
> + for (index = start; index < end; index++) {
> + /*
> + * This is supposed to be the vaddr where the page is being
> + * faulted in, but we have no vaddr here.
> + */
> + struct page *page;
> + unsigned long addr;
> + int avoid_reserve = 0;
> +
> + cond_resched();
> +
> + /*
> + * fallocate(2) manpage permits EINTR; we may have been
> + * interrupted because we are using up too much memory.
> + */
> + if (signal_pending(current)) {
> + error = -EINTR;
> + break;
> + }
> +
> + /* Get policy based on index */
> + pseudo_vma.vm_policy =
> + mpol_shared_policy_lookup(&HUGETLBFS_I(inode)->policy,
> + index);
> +
> + /* addr is the offset within the file (zero based) */
So use loff_t?
> + addr = index * hpage_size;
> +
> + /* mutex taken here, fault path and hole punch */
> + hash = hugetlb_fault_mutex_hash(h, mm, &pseudo_vma, mapping,
> + index, addr);
> + mutex_lock(&hugetlb_fault_mutex_table[hash]);
> +
> + /* See if already present in mapping to avoid alloc/free */
> + page = find_get_page(mapping, index);
> + if (page) {
> + put_page(page);
> + mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> + mpol_cond_put(pseudo_vma.vm_policy);
> + continue;
> + }
> +
> + /* Allocate page and add to page cache */
> + page = alloc_huge_page(&pseudo_vma, addr, avoid_reserve);
> + mpol_cond_put(pseudo_vma.vm_policy);
> + if (IS_ERR(page)) {
> + mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> + error = PTR_ERR(page);
> + goto out;
> + }
> + clear_huge_page(page, addr, pages_per_huge_page(h));
> + __SetPageUptodate(page);
> + error = huge_add_to_page_cache(page, mapping, index);
> + if (unlikely(error)) {
> + put_page(page);
> + mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> + goto out;
> + }
> +
> + mutex_unlock(&hugetlb_fault_mutex_table[hash]);
> +
> + /*
> + * page_put due to reference from alloc_huge_page()
> + * unlock_page because locked by add_to_page_cache()
> + */
> + put_page(page);
> + unlock_page(page);
> + }
> +
> + if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + len > inode->i_size)
> + i_size_write(inode, offset + len);
> + inode->i_ctime = CURRENT_TIME;
> + spin_lock(&inode->i_lock);
> + inode->i_private = NULL;
> + spin_unlock(&inode->i_lock);
> +out:
> + mutex_unlock(&inode->i_mutex);
> + return error;
>
> ...
>
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Andrew Morton @ 2015-07-22 22:06 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Davidlohr Bueso, Aneesh Kumar,
Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On Tue, 21 Jul 2015 11:09:34 -0700 Mike Kravetz <mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> As suggested during the RFC process, tests have been proposed to
> libhugetlbfs as described at:
> http://librelist.com/browser//libhugetlbfs/2015/6/25/patch-tests-add-tests-for-fallocate-system-call/
I didn't know that libhugetlbfs has tests. I wonder if that makes
tools/testing/selftests/vm's hugetlbfstest harmful?
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Davidlohr Bueso @ 2015-07-22 22:19 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Aneesh Kumar, Hillf Danton,
Christoph Hellwig, Michal Hocko
In-Reply-To: <20150722150647.2597c7e5be9ee1eecc438b6f-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
On Wed, 2015-07-22 at 15:06 -0700, Andrew Morton wrote:
> On Tue, 21 Jul 2015 11:09:34 -0700 Mike Kravetz <mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>
> > As suggested during the RFC process, tests have been proposed to
> > libhugetlbfs as described at:
> > http://librelist.com/browser//libhugetlbfs/2015/6/25/patch-tests-add-tests-for-fallocate-system-call/
Great!
>
> I didn't know that libhugetlbfs has tests. I wonder if that makes
> tools/testing/selftests/vm's hugetlbfstest harmful?
Why harmful? Redundant, maybe(?). Does anyone even use selftests for
hugetlbfs regression testing? Lets see, we also have these:
- hugepage-{mmap,shm}.c
- map_hugetlb.c
There's probably a lot of room for improvement here.
^ permalink raw reply
* Re: [PATCH v4 09/10] hugetlbfs: add hugetlbfs_fallocate()
From: Mike Kravetz @ 2015-07-22 22:23 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, linux-api, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Davidlohr Bueso, Aneesh Kumar,
Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <20150722150345.f8d5b0042cfa7112bd95d9ef@linux-foundation.org>
On 07/22/2015 03:03 PM, Andrew Morton wrote:
> On Tue, 21 Jul 2015 11:09:43 -0700 Mike Kravetz <mike.kravetz@oracle.com> wrote:
...
>> +
>> + if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
>> + return -EOPNOTSUPP;
>
> EOPNOTSUPP is a networking thing. It's inappropriate here.
>
> The problem is that if this error is ever returned to userspace, the
> user will be sitting looking at "Operation not supported on transport
> endpoint" and wondering what went wrong in the networking stack.
Trying to follow FALLOCATE(2) man page:
"EOPNOTSUPP
The filesystem containing the file referred to by fd does not
support this operation; or the mode is not supported by the
filesystem containing the file referred to by fd."
--
Mike Kravetz
--
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>
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Andrew Morton @ 2015-07-22 22:30 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: Mike Kravetz, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Aneesh Kumar, Hillf Danton,
Christoph Hellwig, Michal Hocko
In-Reply-To: <1437603594.3298.5.camel-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org>
On Wed, 22 Jul 2015 15:19:54 -0700 Davidlohr Bueso <dave-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org> wrote:
> >
> > I didn't know that libhugetlbfs has tests. I wonder if that makes
> > tools/testing/selftests/vm's hugetlbfstest harmful?
>
> Why harmful? Redundant, maybe(?).
The presence of the in-kernel tests will cause people to add stuff to
them when it would be better if they were to apply that effort to
making libhugetlbfs better. Or vice versa.
Mike's work is an example. Someone later makes a change to hugetlbfs, runs
the kernel selftest and says "yay, everything works", unaware that they
just broke fallocate support.
> Does anyone even use selftests for
> hugetlbfs regression testing? Lets see, we also have these:
>
> - hugepage-{mmap,shm}.c
> - map_hugetlb.c
>
> There's probably a lot of room for improvement here.
selftests is a pretty scrappy place. It's partly a dumping ground for
things so useful test code doesn't just get lost and bitrotted. Partly
a framework so people who add features can easily test them. Partly to
provide tools to architecture maintainers when they wire up new
syscalls and the like.
Unless there's some good reason to retain the hugetlb part of
selftests, I'm thinking we should just remove it to avoid
distracting/misleading people. Or possibly move the libhugetlbfs test
code into the kernel tree and maintain it there.
^ permalink raw reply
* Re: [PATCH v4 09/10] hugetlbfs: add hugetlbfs_fallocate()
From: Andrew Morton @ 2015-07-22 22:30 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Davidlohr Bueso, Aneesh Kumar,
Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <55B017EE.5020203-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On Wed, 22 Jul 2015 15:23:42 -0700 Mike Kravetz <mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> On 07/22/2015 03:03 PM, Andrew Morton wrote:
> > On Tue, 21 Jul 2015 11:09:43 -0700 Mike Kravetz <mike.kravetz-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> ...
> >> +
> >> + if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
> >> + return -EOPNOTSUPP;
> >
> > EOPNOTSUPP is a networking thing. It's inappropriate here.
> >
> > The problem is that if this error is ever returned to userspace, the
> > user will be sitting looking at "Operation not supported on transport
> > endpoint" and wondering what went wrong in the networking stack.
>
> Trying to follow FALLOCATE(2) man page:
>
> "EOPNOTSUPP
> The filesystem containing the file referred to by fd does not
> support this operation; or the mode is not supported by the
> filesystem containing the file referred to by fd."
>
Sigh.
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Davidlohr Bueso @ 2015-07-22 22:34 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Aneesh Kumar, Hillf Danton,
Christoph Hellwig, Michal Hocko
In-Reply-To: <20150722153023.e8f15eb4e490f79cc029c8cd-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
On Wed, 2015-07-22 at 15:30 -0700, Andrew Morton wrote:
> selftests is a pretty scrappy place. It's partly a dumping ground for
> things so useful test code doesn't just get lost and bitrotted. Partly
> a framework so people who add features can easily test them. Partly to
> provide tools to architecture maintainers when they wire up new
> syscalls and the like.
Yeah, ipc, for instance, also sucks _badly_ in selftests.
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Andrew Morton @ 2015-07-22 22:37 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: Mike Kravetz, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Aneesh Kumar, Hillf Danton,
Christoph Hellwig, Michal Hocko
In-Reply-To: <1437604474.3298.7.camel-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org>
On Wed, 22 Jul 2015 15:34:34 -0700 Davidlohr Bueso <dave-h16yJtLeMjHk1uMJSBkQmQ@public.gmane.org> wrote:
> On Wed, 2015-07-22 at 15:30 -0700, Andrew Morton wrote:
> > selftests is a pretty scrappy place. It's partly a dumping ground for
> > things so useful test code doesn't just get lost and bitrotted. Partly
> > a framework so people who add features can easily test them. Partly to
> > provide tools to architecture maintainers when they wire up new
> > syscalls and the like.
>
> Yeah, ipc, for instance, also sucks _badly_ in selftests.
What testsuite should people be using for IPC?
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Davidlohr Bueso @ 2015-07-22 22:50 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, linux-mm, linux-kernel, linux-api, Dave Hansen,
Naoya Horiguchi, David Rientjes, Hugh Dickins, Aneesh Kumar,
Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <20150722153753.08f0a221023706d4ed1cc575@linux-foundation.org>
On Wed, 2015-07-22 at 15:37 -0700, Andrew Morton wrote:
> On Wed, 22 Jul 2015 15:34:34 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote:
>
> > On Wed, 2015-07-22 at 15:30 -0700, Andrew Morton wrote:
> > > selftests is a pretty scrappy place. It's partly a dumping ground for
> > > things so useful test code doesn't just get lost and bitrotted. Partly
> > > a framework so people who add features can easily test them. Partly to
> > > provide tools to architecture maintainers when they wire up new
> > > syscalls and the like.
> >
> > Yeah, ipc, for instance, also sucks _badly_ in selftests.
>
> What testsuite should people be using for IPC?
The best I've found is using the ipc parts of LTP. It's caught a lot of
bugs in the past. Unsurprisingly, I believe Fengguang has this
automated.
Manfred also has a few for more specific purposes -- which also serve
for performance testing:
https://github.com/manfred-colorfu/ipcsemtest
https://github.com/manfred-colorfu/ipcscale
iirc Dave Hansen also had written some shm-specific tests.
--
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>
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Davidlohr Bueso @ 2015-07-22 22:54 UTC (permalink / raw)
To: Andrew Morton
Cc: Mike Kravetz, linux-mm, linux-kernel, linux-api, Dave Hansen,
Naoya Horiguchi, David Rientjes, Hugh Dickins, Aneesh Kumar,
Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <1437605459.3298.14.camel@stgolabs.net>
On Wed, 2015-07-22 at 15:50 -0700, Davidlohr Bueso wrote:
> On Wed, 2015-07-22 at 15:37 -0700, Andrew Morton wrote:
> > On Wed, 22 Jul 2015 15:34:34 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote:
> >
> > > On Wed, 2015-07-22 at 15:30 -0700, Andrew Morton wrote:
> > > > selftests is a pretty scrappy place. It's partly a dumping ground for
> > > > things so useful test code doesn't just get lost and bitrotted. Partly
> > > > a framework so people who add features can easily test them. Partly to
> > > > provide tools to architecture maintainers when they wire up new
> > > > syscalls and the like.
> > >
> > > Yeah, ipc, for instance, also sucks _badly_ in selftests.
> >
> > What testsuite should people be using for IPC?
>
> The best I've found is using the ipc parts of LTP. It's caught a lot of
> bugs in the past. Unsurprisingly, I believe Fengguang has this
> automated.
>
> Manfred also has a few for more specific purposes -- which also serve
> for performance testing:
> https://github.com/manfred-colorfu/ipcsemtest
> https://github.com/manfred-colorfu/ipcscale
>
> iirc Dave Hansen also had written some shm-specific tests.
I also used to rely on multiple Oracle 11g RDBMS workloads and
benchmarks, particularly for dealing with sysv sems. Unfortunately I no
longer have the required licenses :(
--
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>
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Mike Kravetz @ 2015-07-22 23:18 UTC (permalink / raw)
To: Andrew Morton, Davidlohr Bueso, Eric B Munson
Cc: linux-mm, linux-kernel, linux-api, Dave Hansen, Naoya Horiguchi,
David Rientjes, Hugh Dickins, Aneesh Kumar, Hillf Danton,
Christoph Hellwig, Michal Hocko
In-Reply-To: <20150722153023.e8f15eb4e490f79cc029c8cd@linux-foundation.org>
On 07/22/2015 03:30 PM, Andrew Morton wrote:
> On Wed, 22 Jul 2015 15:19:54 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote:
>
>>>
>>> I didn't know that libhugetlbfs has tests. I wonder if that makes
>>> tools/testing/selftests/vm's hugetlbfstest harmful?
>>
>> Why harmful? Redundant, maybe(?).
>
> The presence of the in-kernel tests will cause people to add stuff to
> them when it would be better if they were to apply that effort to
> making libhugetlbfs better. Or vice versa.
>
> Mike's work is an example. Someone later makes a change to hugetlbfs, runs
> the kernel selftest and says "yay, everything works", unaware that they
> just broke fallocate support.
>
>> Does anyone even use selftests for
>> hugetlbfs regression testing? Lets see, we also have these:
>>
>> - hugepage-{mmap,shm}.c
>> - map_hugetlb.c
>>
>> There's probably a lot of room for improvement here.
>
> selftests is a pretty scrappy place. It's partly a dumping ground for
> things so useful test code doesn't just get lost and bitrotted. Partly
> a framework so people who add features can easily test them. Partly to
> provide tools to architecture maintainers when they wire up new
> syscalls and the like.
>
> Unless there's some good reason to retain the hugetlb part of
> selftests, I'm thinking we should just remove it to avoid
> distracting/misleading people. Or possibly move the libhugetlbfs test
> code into the kernel tree and maintain it there.
Adding Eric as he is the libhugetlbfs maintainer.
I think removing the hugetlb selftests in the kernel and pointing
people to libhugetlbfs is the way to go. From a very quick scan
of the selftests, I would guess libhugetlbfs covers everything
in those tests.
I'm willing to verify the testing provided by selftests is included
in libhugetlbfs, and remove selftests if that is the direction we
want to take.
--
Mike Kravetz
--
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>
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Davidlohr Bueso @ 2015-07-22 23:54 UTC (permalink / raw)
To: Mike Kravetz
Cc: Andrew Morton, Eric B Munson, linux-mm, linux-kernel, linux-api,
Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Aneesh Kumar, Hillf Danton, Christoph Hellwig, Michal Hocko
In-Reply-To: <55B024C6.8010504@oracle.com>
On Wed, 2015-07-22 at 16:18 -0700, Mike Kravetz wrote:
> On 07/22/2015 03:30 PM, Andrew Morton wrote:
> > On Wed, 22 Jul 2015 15:19:54 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote:
> >
> >>>
> >>> I didn't know that libhugetlbfs has tests. I wonder if that makes
> >>> tools/testing/selftests/vm's hugetlbfstest harmful?
> >>
> >> Why harmful? Redundant, maybe(?).
> >
> > The presence of the in-kernel tests will cause people to add stuff to
> > them when it would be better if they were to apply that effort to
> > making libhugetlbfs better. Or vice versa.
> >
> > Mike's work is an example. Someone later makes a change to hugetlbfs, runs
> > the kernel selftest and says "yay, everything works", unaware that they
> > just broke fallocate support.
> >
> >> Does anyone even use selftests for
> >> hugetlbfs regression testing? Lets see, we also have these:
> >>
> >> - hugepage-{mmap,shm}.c
> >> - map_hugetlb.c
> >>
> >> There's probably a lot of room for improvement here.
> >
> > selftests is a pretty scrappy place. It's partly a dumping ground for
> > things so useful test code doesn't just get lost and bitrotted. Partly
> > a framework so people who add features can easily test them. Partly to
> > provide tools to architecture maintainers when they wire up new
> > syscalls and the like.
> >
> > Unless there's some good reason to retain the hugetlb part of
> > selftests, I'm thinking we should just remove it to avoid
> > distracting/misleading people. Or possibly move the libhugetlbfs test
> > code into the kernel tree and maintain it there.
>
> Adding Eric as he is the libhugetlbfs maintainer.
>
> I think removing the hugetlb selftests in the kernel and pointing
> people to libhugetlbfs is the way to go. From a very quick scan
> of the selftests, I would guess libhugetlbfs covers everything
> in those tests.
fwiw, I've been trying to push people towards this for a while. Ie:
commit 15610c86fa83ff778eb80d3cfaa71d6acceb628a
Author: Davidlohr Bueso <davidlohr@hp.com>
Date: Wed Sep 11 14:21:48 2013 -0700
hugepage: mention libhugetlbfs in doc
Explicitly mention/recommend using the libhugetlbfs test cases when
changing related kernel code. Developers that are unaware of the project
can easily miss this and introduce potential regressions that may or may
not be caught by community review.
Also do some cleanups that make the document visually easier to view at a
first glance.
Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
But generally speaking, I doubt this doc is read much.
>
> I'm willing to verify the testing provided by selftests is included
> in libhugetlbfs, and remove selftests if that is the direction we
> want to take.
Ack to this idea and thanks for volunteering.
--
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>
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: David Rientjes @ 2015-07-23 0:05 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: Mike Kravetz, Andrew Morton, Eric B Munson, linux-mm,
linux-kernel, linux-api, Dave Hansen, Naoya Horiguchi,
Hugh Dickins, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Michal Hocko
In-Reply-To: <1437609241.3298.19.camel@stgolabs.net>
On Wed, 22 Jul 2015, Davidlohr Bueso wrote:
> fwiw, I've been trying to push people towards this for a while. Ie:
>
> commit 15610c86fa83ff778eb80d3cfaa71d6acceb628a
> Author: Davidlohr Bueso <davidlohr@hp.com>
> Date: Wed Sep 11 14:21:48 2013 -0700
>
> hugepage: mention libhugetlbfs in doc
>
> Explicitly mention/recommend using the libhugetlbfs test cases when
> changing related kernel code. Developers that are unaware of the project
> can easily miss this and introduce potential regressions that may or may
> not be caught by community review.
>
> Also do some cleanups that make the document visually easier to view at a
> first glance.
>
> Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>
> But generally speaking, I doubt this doc is read much.
>
The mmap(2) man page cites it specifically as the source of information on
MAP_HUGETLB and it, in turn, directs people to
tools/testing/selftests/vm/map_hugetlb.c. It also mentions libhugetlbfs
as a result of your patch, so perhaps change the man page to point people
directly there?
--
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>
^ permalink raw reply
* Re: [PATCH V4 2/6] mm: mlock: Add new mlock, munlock, and munlockall system calls
From: Ralf Baechle @ 2015-07-23 6:58 UTC (permalink / raw)
To: Eric B Munson
Cc: Michael Ellerman, Andrew Morton, linux-mips, linux-m68k,
linux-ia64, linux-sh, Catalin Marinas, Heiko Carstens,
Michal Hocko, linux-mm, sparclinux, linux-arch, Stephen Rothwell,
linux-am33-list, Geert Uytterhoeven, Vlastimil Babka,
Guenter Roeck, linux-xtensa, linux-s390, adi-buildroot-devel,
linux-arm-kernel, linux-cris-kernel, linux-parisc
In-Reply-To: <20150722141501.GA3203@akamai.com>
On Wed, Jul 22, 2015 at 10:15:01AM -0400, Eric B Munson wrote:
> >
> > You haven't wired it up properly on powerpc, but I haven't mentioned it because
> > I'd rather we did it.
> >
> > cheers
>
> It looks like I will be spinning a V5, so I will drop all but the x86
> system calls additions in that version.
The MIPS bits are looking good however, so
Acked-by: Ralf Baechle <ralf@linux-mips.org>
With my ack, will you keep them or maybe carry them as a separate patch?
Cheers,
Ralf
--
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>
^ permalink raw reply
* Re: [PATCH -mm v9 7/8] proc: export idle flag via kpageflags
From: Vladimir Davydov @ 2015-07-23 7:57 UTC (permalink / raw)
To: Andrew Morton, Andres Lagar-Cavilla
Cc: Minchan Kim, Raghavendra K T, Johannes Weiner, Michal Hocko,
Greg Thelen, Michel Lespinasse, David Rientjes, Pavel Emelyanov,
Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
cgroups, linux-kernel
In-Reply-To: <CAJu=L5-QKjd8ZjsUY8xrrtVB0k=aK5HSQAvscmqRjoJapj3_-A@mail.gmail.com>
On Wed, Jul 22, 2015 at 01:46:21PM -0700, Andres Lagar-Cavilla wrote:
> In page_referenced_one:
>
> + if (referenced)
> + clear_page_idle(page);
>
Yep, that's it. Thanks, Andres.
^ permalink raw reply
* Re: [PATCH V4 4/6] mm: mlock: Introduce VM_LOCKONFAULT and add mlock flags to enable it
From: Vlastimil Babka @ 2015-07-23 10:03 UTC (permalink / raw)
To: Eric B Munson
Cc: Andrew Morton, Michal Hocko, Jonathan Corbet, linux-alpha,
linux-kernel, linux-mips, linux-parisc, linuxppc-dev, sparclinux,
linux-xtensa, dri-devel, linux-mm, linux-arch, linux-api
In-Reply-To: <20150722184343.GA2351@akamai.com>
On 07/22/2015 08:43 PM, Eric B Munson wrote:
> On Wed, 22 Jul 2015, Vlastimil Babka wrote:
>
>>
>> Hi,
>>
>> I think you should include a complete description of which
>> transitions for vma states and mlock2/munlock2 flags applied on them
>> are valid and what they do. It will also help with the manpages.
>> You explained some to Jon in the last thread, but I think there
>> should be a canonical description in changelog (if not also
>> Documentation, if mlock is covered there).
>>
>> For example the scenario Jon asked, what happens after a
>> mlock2(MLOCK_ONFAULT) followed by mlock2(MLOCK_LOCKED), and that the
>> answer is "nothing". Your promised code comment for
>> apply_vma_flags() doesn't suffice IMHO (and I'm not sure it's there,
>> anyway?).
>
> I missed adding that comment to the code, will be there in V5 along with
> the description in the changelog.
Thanks!
>>
>> But the more I think about the scenario and your new VM_LOCKONFAULT
>> vma flag, it seems awkward to me. Why should munlocking at all care
>> if the vma was mlocked with MLOCK_LOCKED or MLOCK_ONFAULT? In either
>> case the result is that all pages currently populated are munlocked.
>> So the flags for munlock2 should be unnecessary.
>
> Say a user has a large area of interleaved MLOCK_LOCK and MLOCK_ONFAULT
> mappings and they want to unlock only the ones with MLOCK_LOCK. With
> the current implementation, this is possible in a single system call
> that spans the entire region. With your suggestion, the user would have
> to know what regions where locked with MLOCK_LOCK and call munlock() on
> each of them. IMO, the way munlock2() works better mirrors the way
> munlock() currently works when called on a large area of interleaved
> locked and unlocked areas.
Um OK, that scenario is possible in theory. But I have a hard time imagining
that somebody would really want to do that. I think much more people would
benefit from a simpler API.
>
>>
>> I also think VM_LOCKONFAULT is unnecessary. VM_LOCKED should be
>> enough - see how you had to handle the new flag in all places that
>> had to handle the old flag? I think the information whether mlock
>> was supposed to fault the whole vma is obsolete at the moment mlock
>> returns. VM_LOCKED should be enough for both modes, and the flag to
>> mlock2 could just control whether the pre-faulting is done.
>>
>> So what should be IMHO enough:
>> - munlock can stay without flags
>> - mlock2 has only one new flag MLOCK_ONFAULT. If specified,
>> pre-faulting is not done, just set VM_LOCKED and mlock pages already
>> present.
>> - same with mmap(MAP_LOCKONFAULT) (need to define what happens when
>> both MAP_LOCKED and MAP_LOCKONFAULT are specified).
>>
>> Now mlockall(MCL_FUTURE) muddles the situation in that it stores the
>> information for future VMA's in current->mm->def_flags, and this
>> def_flags would need to distinguish VM_LOCKED with population and
>> without. But that could be still solvable without introducing a new
>> vma flag everywhere.
>
> With you right up until that last paragraph. I have been staring at
> this a while and I cannot come up a way to handle the
> mlockall(MCL_ONFAULT) without introducing a new vm flag. It doesn't
> have to be VM_LOCKONFAULT, we could use the model that Michal Hocko
> suggested with something like VM_FAULTPOPULATE. However, we can't
> really use this flag anywhere except the mlock code becuase we have to
> be able to distinguish a caller that wants to use MLOCK_LOCK with
> whatever control VM_FAULTPOPULATE might grant outside of mlock and a
> caller that wants MLOCK_ONFAULT. That was a long way of saying we need
> an extra vma flag regardless. However, if that flag only controls if
> mlock pre-populates it would work and it would do away with most of the
> places I had to touch to handle VM_LOCKONFAULT properly.
Yes, it would be a good way. Adding a new vma flag is probably cleanest after
all, but the flag would be set *in addition* to VM_LOCKED, *just* to prevent
pre-faulting. The places that check VM_LOCKED for the actual page mlocking (i.e.
try_to_unmap_one) would just keep checking VM_LOCKED. The places where VM_LOCKED
is checked to trigger prepopulation, would skip that if VM_LOCKONFAULT is also
set. Having VM_LOCKONFAULT set without also VM_LOCKED itself would be invalid state.
This should work fine with the simplified API as I proposed so let me reiterate
and try fill in the blanks:
- mlock2 has only one new flag MLOCK_ONFAULT. If specified, VM_LOCKONFAULT is
set in addition to VM_LOCKED and no prefaulting is done
- old mlock syscall naturally behaves as mlock2 without MLOCK_ONFAULT
- calling mlock/mlock2 on an already-mlocked area (if that's permitted
already?) will add/remove VM_LOCKONFAULT as needed. If it's removing,
prepopulate whole range. Of course adding VM_LOCKONFAULT to a vma that was
already prefaulted doesn't make any difference, but it's consistent with the rest.
- munlock removes both VM_LOCKED and VM_LOCKONFAULT
- mmap could treat MAP_LOCKONFAULT as a modifier to MAP_LOCKED to be consistent?
or not? I'm not sure here, either way subtly differs from mlock API anyway, I
just wish MAP_LOCKED never existed...
- mlockall(MCL_CURRENT) sets or clears VM_LOCKONFAULT depending on
MCL_LOCKONFAULT, mlockall(MCL_FUTURE) does the same on mm->def_flags
- munlockall2 removes both, like munlock. munlockall2(MCL_FUTURE) does that to
def_flags
> I picked VM_LOCKONFAULT because it is explicit about what it is for and
> there is little risk of someone coming along in 5 years and saying "why
> not overload this flag to do this other thing completely unrelated to
> mlock?". A flag for controling speculative population is more likely to
> be overloaded outside of mlock().
Sure, let's make clear the name is related to mlock, but the behavior could
still be additive to MAP_LOCKED.
> If you have a sane way of handling mlockall(MCL_ONFAULT) without a new
> VMA flag, I am happy to give it a try, but I haven't been able to come
> up with one that doesn't have its own gremlins.
Well we could store the MCL_FUTURE | MCL_ONFAULT bit elsewhere in mm_struct than
the def_flags field. The VM_LOCKED field is already evaluated specially from all
the other def_flags. We are nearing the full 32bit space for vma flags. I think
all I've proposed above wouldn't change much if we removed per-vma
VM_LOCKONFAULT flag from the equation. Just that re-mlocking area already
mlocked *withouth* MLOCK_ONFAULT wouldn't know that it was alread prepopulated,
and would have to re-populate in either case (I'm not sure, maybe it's already
done by current implementation anyway so it's not a potential performance
regression).
Only mlockall(MCL_FUTURE | MCL_ONFAULT) should really need the ONFAULT info to
"stick" somewhere in mm_struct, but it doesn't have to be def_flags?
^ permalink raw reply
* Re: [PATCH v4 00/10] hugetlbfs: add fallocate support
From: Eric B Munson @ 2015-07-23 15:17 UTC (permalink / raw)
To: Mike Kravetz
Cc: Andrew Morton, Davidlohr Bueso, linux-mm, linux-kernel, linux-api,
Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Aneesh Kumar, Hillf Danton, Christoph Hellwig, Michal Hocko,
emunson
In-Reply-To: <55B024C6.8010504@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]
On Wed, 22 Jul 2015, Mike Kravetz wrote:
> On 07/22/2015 03:30 PM, Andrew Morton wrote:
> >On Wed, 22 Jul 2015 15:19:54 -0700 Davidlohr Bueso <dave@stgolabs.net> wrote:
> >
> >>>
> >>>I didn't know that libhugetlbfs has tests. I wonder if that makes
> >>>tools/testing/selftests/vm's hugetlbfstest harmful?
> >>
> >>Why harmful? Redundant, maybe(?).
> >
> >The presence of the in-kernel tests will cause people to add stuff to
> >them when it would be better if they were to apply that effort to
> >making libhugetlbfs better. Or vice versa.
> >
> >Mike's work is an example. Someone later makes a change to hugetlbfs, runs
> >the kernel selftest and says "yay, everything works", unaware that they
> >just broke fallocate support.
> >
> >>Does anyone even use selftests for
> >>hugetlbfs regression testing? Lets see, we also have these:
> >>
> >>- hugepage-{mmap,shm}.c
> >>- map_hugetlb.c
> >>
> >>There's probably a lot of room for improvement here.
> >
> >selftests is a pretty scrappy place. It's partly a dumping ground for
> >things so useful test code doesn't just get lost and bitrotted. Partly
> >a framework so people who add features can easily test them. Partly to
> >provide tools to architecture maintainers when they wire up new
> >syscalls and the like.
> >
> >Unless there's some good reason to retain the hugetlb part of
> >selftests, I'm thinking we should just remove it to avoid
> >distracting/misleading people. Or possibly move the libhugetlbfs test
> >code into the kernel tree and maintain it there.
>
> Adding Eric as he is the libhugetlbfs maintainer.
>
> I think removing the hugetlb selftests in the kernel and pointing
> people to libhugetlbfs is the way to go. From a very quick scan
> of the selftests, I would guess libhugetlbfs covers everything
> in those tests.
>
> I'm willing to verify the testing provided by selftests is included
> in libhugetlbfs, and remove selftests if that is the direction we
> want to take.
I would rather see the test suite stay in the library, there are a
number of tests that rely on infrastructure in the library that is not
available in selftests.
I am happy to help with any tests that need to be added/modified in the
library to cover.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v8 8/9] nvmem: sunxi: Move the SID driver to the nvmem framework
From: Stefan Wahren @ 2015-07-23 15:18 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Greg Kroah-Hartman, devicetree-u79uwXL29TY76Z2rM5mHXA,
arnd-r2nGTMty4D4, linux-api-u79uwXL29TY76Z2rM5mHXA,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w, Mark Brown,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
mporter-OWPKS81ov/FWk0Htik3J/w, Maxime Ripard,
wxt-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <1437403462-4432-1-git-send-email-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Hi Srinivas,
Am 20.07.2015 um 16:44 schrieb Srinivas Kandagatla:
> From: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>
> Now that we have the nvmem framework, we can consolidate the common
> driver code. Move the driver to the framework, and hopefully, it will
> fix the sysfs file creation race.
>
> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> [srinivas.kandagatla: Moved to regmap based EEPROM framework]
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 ---
> .../bindings/misc/allwinner,sunxi-sid.txt | 17 ---
> .../bindings/nvmem/allwinner,sunxi-sid.txt | 21 +++
> drivers/misc/eeprom/Kconfig | 13 --
> drivers/misc/eeprom/Makefile | 1 -
> drivers/misc/eeprom/sunxi_sid.c | 156 --------------------
> drivers/nvmem/Kconfig | 11 ++
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/sunxi_sid.c | 159 +++++++++++++++++++++
> 9 files changed, 193 insertions(+), 209 deletions(-)
> delete mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
> delete mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
> create mode 100644 Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
> delete mode 100644 drivers/misc/eeprom/sunxi_sid.c
> create mode 100644 drivers/nvmem/sunxi_sid.c
>
> [...]
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index ff44fe9..4328b93 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -8,3 +8,5 @@ nvmem_core-y := core.o
> # Devices
> obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
> nvmem_qfprom-y := qfprom.o
> +obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem_sunxi_sid.o
> +nvmem_sunxi_sid-y := sunxi_sid.o
is it really necessary to have 2 lines for a single driver?
Why not the following line?
obj-$(CONFIG_NVMEM_SUNXI_SID) += sunxi_sid.o
Regards
Stefan
^ permalink raw reply
* Re: [PATCH V4 4/6] mm: mlock: Introduce VM_LOCKONFAULT and add mlock flags to enable it
From: Eric B Munson @ 2015-07-23 15:21 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, Michal Hocko, Jonathan Corbet, linux-alpha,
linux-kernel, linux-mips, linux-parisc, linuxppc-dev, sparclinux,
linux-xtensa, dri-devel, linux-mm, linux-arch, linux-api
In-Reply-To: <55B0BBF9.7050802@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 7676 bytes --]
On Thu, 23 Jul 2015, Vlastimil Babka wrote:
> On 07/22/2015 08:43 PM, Eric B Munson wrote:
> > On Wed, 22 Jul 2015, Vlastimil Babka wrote:
> >
> >>
> >> Hi,
> >>
> >> I think you should include a complete description of which
> >> transitions for vma states and mlock2/munlock2 flags applied on them
> >> are valid and what they do. It will also help with the manpages.
> >> You explained some to Jon in the last thread, but I think there
> >> should be a canonical description in changelog (if not also
> >> Documentation, if mlock is covered there).
> >>
> >> For example the scenario Jon asked, what happens after a
> >> mlock2(MLOCK_ONFAULT) followed by mlock2(MLOCK_LOCKED), and that the
> >> answer is "nothing". Your promised code comment for
> >> apply_vma_flags() doesn't suffice IMHO (and I'm not sure it's there,
> >> anyway?).
> >
> > I missed adding that comment to the code, will be there in V5 along with
> > the description in the changelog.
>
> Thanks!
>
> >>
> >> But the more I think about the scenario and your new VM_LOCKONFAULT
> >> vma flag, it seems awkward to me. Why should munlocking at all care
> >> if the vma was mlocked with MLOCK_LOCKED or MLOCK_ONFAULT? In either
> >> case the result is that all pages currently populated are munlocked.
> >> So the flags for munlock2 should be unnecessary.
> >
> > Say a user has a large area of interleaved MLOCK_LOCK and MLOCK_ONFAULT
> > mappings and they want to unlock only the ones with MLOCK_LOCK. With
> > the current implementation, this is possible in a single system call
> > that spans the entire region. With your suggestion, the user would have
> > to know what regions where locked with MLOCK_LOCK and call munlock() on
> > each of them. IMO, the way munlock2() works better mirrors the way
> > munlock() currently works when called on a large area of interleaved
> > locked and unlocked areas.
>
> Um OK, that scenario is possible in theory. But I have a hard time imagining
> that somebody would really want to do that. I think much more people would
> benefit from a simpler API.
It wasn't about imagining a scenario, more about keeping parity with
something that currently works (unlocking a large area of interleaved
locked and unlocked regions). However, there is no reason we can't add
the new munlock2 later if it is desired.
>
> >
> >>
> >> I also think VM_LOCKONFAULT is unnecessary. VM_LOCKED should be
> >> enough - see how you had to handle the new flag in all places that
> >> had to handle the old flag? I think the information whether mlock
> >> was supposed to fault the whole vma is obsolete at the moment mlock
> >> returns. VM_LOCKED should be enough for both modes, and the flag to
> >> mlock2 could just control whether the pre-faulting is done.
> >>
> >> So what should be IMHO enough:
> >> - munlock can stay without flags
> >> - mlock2 has only one new flag MLOCK_ONFAULT. If specified,
> >> pre-faulting is not done, just set VM_LOCKED and mlock pages already
> >> present.
> >> - same with mmap(MAP_LOCKONFAULT) (need to define what happens when
> >> both MAP_LOCKED and MAP_LOCKONFAULT are specified).
> >>
> >> Now mlockall(MCL_FUTURE) muddles the situation in that it stores the
> >> information for future VMA's in current->mm->def_flags, and this
> >> def_flags would need to distinguish VM_LOCKED with population and
> >> without. But that could be still solvable without introducing a new
> >> vma flag everywhere.
> >
> > With you right up until that last paragraph. I have been staring at
> > this a while and I cannot come up a way to handle the
> > mlockall(MCL_ONFAULT) without introducing a new vm flag. It doesn't
> > have to be VM_LOCKONFAULT, we could use the model that Michal Hocko
> > suggested with something like VM_FAULTPOPULATE. However, we can't
> > really use this flag anywhere except the mlock code becuase we have to
> > be able to distinguish a caller that wants to use MLOCK_LOCK with
> > whatever control VM_FAULTPOPULATE might grant outside of mlock and a
> > caller that wants MLOCK_ONFAULT. That was a long way of saying we need
> > an extra vma flag regardless. However, if that flag only controls if
> > mlock pre-populates it would work and it would do away with most of the
> > places I had to touch to handle VM_LOCKONFAULT properly.
>
> Yes, it would be a good way. Adding a new vma flag is probably cleanest after
> all, but the flag would be set *in addition* to VM_LOCKED, *just* to prevent
> pre-faulting. The places that check VM_LOCKED for the actual page mlocking (i.e.
> try_to_unmap_one) would just keep checking VM_LOCKED. The places where VM_LOCKED
> is checked to trigger prepopulation, would skip that if VM_LOCKONFAULT is also
> set. Having VM_LOCKONFAULT set without also VM_LOCKED itself would be invalid state.
>
> This should work fine with the simplified API as I proposed so let me reiterate
> and try fill in the blanks:
>
> - mlock2 has only one new flag MLOCK_ONFAULT. If specified, VM_LOCKONFAULT is
> set in addition to VM_LOCKED and no prefaulting is done
> - old mlock syscall naturally behaves as mlock2 without MLOCK_ONFAULT
> - calling mlock/mlock2 on an already-mlocked area (if that's permitted
> already?) will add/remove VM_LOCKONFAULT as needed. If it's removing,
> prepopulate whole range. Of course adding VM_LOCKONFAULT to a vma that was
> already prefaulted doesn't make any difference, but it's consistent with the rest.
> - munlock removes both VM_LOCKED and VM_LOCKONFAULT
> - mmap could treat MAP_LOCKONFAULT as a modifier to MAP_LOCKED to be consistent?
> or not? I'm not sure here, either way subtly differs from mlock API anyway, I
> just wish MAP_LOCKED never existed...
> - mlockall(MCL_CURRENT) sets or clears VM_LOCKONFAULT depending on
> MCL_LOCKONFAULT, mlockall(MCL_FUTURE) does the same on mm->def_flags
> - munlockall2 removes both, like munlock. munlockall2(MCL_FUTURE) does that to
> def_flags
>
> > I picked VM_LOCKONFAULT because it is explicit about what it is for and
> > there is little risk of someone coming along in 5 years and saying "why
> > not overload this flag to do this other thing completely unrelated to
> > mlock?". A flag for controling speculative population is more likely to
> > be overloaded outside of mlock().
>
> Sure, let's make clear the name is related to mlock, but the behavior could
> still be additive to MAP_LOCKED.
>
> > If you have a sane way of handling mlockall(MCL_ONFAULT) without a new
> > VMA flag, I am happy to give it a try, but I haven't been able to come
> > up with one that doesn't have its own gremlins.
>
> Well we could store the MCL_FUTURE | MCL_ONFAULT bit elsewhere in mm_struct than
> the def_flags field. The VM_LOCKED field is already evaluated specially from all
> the other def_flags. We are nearing the full 32bit space for vma flags. I think
> all I've proposed above wouldn't change much if we removed per-vma
> VM_LOCKONFAULT flag from the equation. Just that re-mlocking area already
> mlocked *withouth* MLOCK_ONFAULT wouldn't know that it was alread prepopulated,
> and would have to re-populate in either case (I'm not sure, maybe it's already
> done by current implementation anyway so it's not a potential performance
> regression).
> Only mlockall(MCL_FUTURE | MCL_ONFAULT) should really need the ONFAULT info to
> "stick" somewhere in mm_struct, but it doesn't have to be def_flags?
This all sounds fine and should still cover the usecase that started
this adventure. I will include this change in the V5 spin.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v8 1/9] nvmem: Add a simple NVMEM framework for nvmem providers
From: Stefan Wahren @ 2015-07-23 15:26 UTC (permalink / raw)
To: Srinivas Kandagatla, linux-arm-kernel, Greg Kroah-Hartman
Cc: Rob Herring, Mark Brown, s.hauer, linux-api, linux-kernel,
devicetree, linux-arm-msm, arnd, sboyd, pantelis.antoniou,
mporter, wxt, Maxime Ripard
In-Reply-To: <1437403392-4136-1-git-send-email-srinivas.kandagatla@linaro.org>
Hi Srinivas,
Am 20.07.2015 um 16:43 schrieb Srinivas Kandagatla:
> This patch adds just providers part of the framework just to enable easy
> review.
>
> Up until now, NVMEM drivers like eeprom were stored in drivers/misc,
> where they all had to duplicate pretty much the same code to register
> a sysfs file, allow in-kernel users to access the content of the devices
> they were driving, etc.
>
> This was also a problem as far as other in-kernel users were involved,
> since the solutions used were pretty much different from on driver to
> another, there was a rather big abstraction leak.
>
> This introduction of this framework aims at solving this. It also
> introduces DT representation for consumer devices to go get the data
> they require (MAC Addresses, SoC/Revision ID, part numbers, and so on)
> from the nvmems.
>
> Having regmap interface to this framework would give much better
> abstraction for nvmems on different buses.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> [Maxime Ripard: intial version of eeprom framework]
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/nvmem/Kconfig | 13 ++
> drivers/nvmem/Makefile | 6 +
> drivers/nvmem/core.c | 384 +++++++++++++++++++++++++++++++++++++++++
> include/linux/nvmem-consumer.h | 23 +++
> include/linux/nvmem-provider.h | 47 +++++
> 7 files changed, 476 insertions(+)
> create mode 100644 drivers/nvmem/Kconfig
> create mode 100644 drivers/nvmem/Makefile
> create mode 100644 drivers/nvmem/core.c
> create mode 100644 include/linux/nvmem-consumer.h
> create mode 100644 include/linux/nvmem-provider.h
i've tested this patch with my mxs-ocotp driver [1].
So you can add
Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
Regards
Stefan
^ permalink raw reply
* Re: [PATCH v8 8/9] nvmem: sunxi: Move the SID driver to the nvmem framework
From: Srinivas Kandagatla @ 2015-07-23 15:48 UTC (permalink / raw)
To: Stefan Wahren, Greg Kroah-Hartman,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: wxt-TNX95d0MmH7DzftRWevZcw, linux-api-u79uwXL29TY76Z2rM5mHXA,
Rob Herring, sboyd-sgV2jX0FEOL9JmXXK+q4OQ, arnd-r2nGTMty4D4,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
mporter-OWPKS81ov/FWk0Htik3J/w,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w, Mark Brown,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2114639619.71276.1437496724946.JavaMail.open-xchange-0SF9iQWekqLc2Cnrm9MUdsgmgJlYmuWJ@public.gmane.org>
On 21/07/15 17:38, Stefan Wahren wrote:
> Hi Srinivas,
>
>> Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> hat am 20. Juli 2015 um
>> 16:44 geschrieben:
>>
>>
>> From: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>
>> Now that we have the nvmem framework, we can consolidate the common
>> driver code. Move the driver to the framework, and hopefully, it will
>> fix the sysfs file creation race.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> [srinivas.kandagatla: Moved to regmap based EEPROM framework]
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 ---
>> .../bindings/misc/allwinner,sunxi-sid.txt | 17 ---
>> .../bindings/nvmem/allwinner,sunxi-sid.txt | 21 +++
>> drivers/misc/eeprom/Kconfig | 13 --
>> drivers/misc/eeprom/Makefile | 1 -
>> drivers/misc/eeprom/sunxi_sid.c | 156 --------------------
>> drivers/nvmem/Kconfig | 11 ++
>> drivers/nvmem/Makefile | 2 +
>> drivers/nvmem/sunxi_sid.c | 159 +++++++++++++++++++++
>> 9 files changed, 193 insertions(+), 209 deletions(-)
>> delete mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
>> delete mode 100644
>> Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
>> create mode 100644
>> Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
>> delete mode 100644 drivers/misc/eeprom/sunxi_sid.c
>> create mode 100644 drivers/nvmem/sunxi_sid.c
>>
>> [...]
>> -static int sunxi_sid_probe(struct platform_device *pdev)
>> -{
>> - struct sunxi_sid_data *sid_data;
>> - struct resource *res;
>> - const struct of_device_id *of_dev_id;
>> - u8 *entropy;
>> - unsigned int i;
>> -
>> - sid_data = devm_kzalloc(&pdev->dev, sizeof(struct sunxi_sid_data),
>> - GFP_KERNEL);
>> - if (!sid_data)
>> - return -ENOMEM;
>> -
>> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> - sid_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
>> - if (IS_ERR(sid_data->reg_base))
>> - return PTR_ERR(sid_data->reg_base);
>> -
>> - of_dev_id = of_match_device(sunxi_sid_of_match, &pdev->dev);
>> - if (!of_dev_id)
>> - return -ENODEV;
>> - sid_data->keysize = (int)of_dev_id->data;
>> -
>> - platform_set_drvdata(pdev, sid_data);
>> -
>> - sid_bin_attr.size = sid_data->keysize;
>> - if (device_create_bin_file(&pdev->dev, &sid_bin_attr))
>> - return -ENODEV;
>> -
>> - entropy = kzalloc(sizeof(u8) * sid_data->keysize, GFP_KERNEL);
>> - for (i = 0; i < sid_data->keysize; i++)
>> - entropy[i] = sunxi_sid_read_byte(sid_data, i);
>> - add_device_randomness(entropy, sid_data->keysize);
>> - kfree(entropy);
>> -
>
> in case of porting a driver to a new framework, i would expect the same
> features.
> The ported driver do not add the Security ID to the device randomness.
>
> What's the reason for this difference?
Not sure why it was discarded in the first place, but I can put it back
which will atleast add some data to random pool.
--srini
>
> Regards
> Stefan
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox