From: Mel Gorman <mgorman@suse.de> To: Jan Kara <jack@suse.cz> Cc: linux-mm@kvack.org, linux-media@vger.kernel.org, Hans Verkuil <hverkuil@xs4all.nl>, dri-devel@lists.freedesktop.org, Pawel Osciak <pawel@osciak.com>, Mauro Carvalho Chehab <mchehab@osg.samsung.com>, Marek Szyprowski <m.szyprowski@samsung.com>, linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 2/9] mm: Provide new get_vaddr_frames() helper Date: Mon, 11 May 2015 15:47:07 +0100 [thread overview] Message-ID: <20150511144707.GP2462@suse.de> (raw) In-Reply-To: <20150511140019.GD25034@quack.suse.cz> On Mon, May 11, 2015 at 04:00:19PM +0200, Jan Kara wrote: > > > +int get_vaddr_frames(unsigned long start, unsigned int nr_frames, > > > + bool write, bool force, struct frame_vector *vec) > > > +{ > > > + struct mm_struct *mm = current->mm; > > > + struct vm_area_struct *vma; > > > + int ret = 0; > > > + int err; > > > + int locked = 1; > > > + > > > > bool locked. > It cannot be bool. It is passed to get_user_pages_locked() which expects > int *. > My bad. > > > +int frame_vector_to_pages(struct frame_vector *vec) > > > +{ > > > > I think it's probably best to make the relevant counters in frame_vector > > signed and limit the maximum possible size of it. It's still not putting > > any practical limit on the size of the frame_vector. > > I don't see a reason why counters in frame_vector should be signed... Can > you share your reason? I've added a check into frame_vector_create() to > limit number of frames to INT_MAX / sizeof(void *) / 2 to avoid arithmetics > overflow. Thanks for review! > Only that the return value of frame_vector_to_pages() returns int where as the potential range that is converted is unsigned int. I don't think there are any mistakes dealing with signed/unsigned but I don't see any advantage of using unsigned either and limiting it to INT_MAX either. It's not a big deal. -- Mel Gorman SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@suse.de> To: Jan Kara <jack@suse.cz> Cc: linux-mm@kvack.org, linux-media@vger.kernel.org, Hans Verkuil <hverkuil@xs4all.nl>, dri-devel@lists.freedesktop.org, Pawel Osciak <pawel@osciak.com>, Mauro Carvalho Chehab <mchehab@osg.samsung.com>, Marek Szyprowski <m.szyprowski@samsung.com>, linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 2/9] mm: Provide new get_vaddr_frames() helper Date: Mon, 11 May 2015 15:47:07 +0100 [thread overview] Message-ID: <20150511144707.GP2462@suse.de> (raw) In-Reply-To: <20150511140019.GD25034@quack.suse.cz> On Mon, May 11, 2015 at 04:00:19PM +0200, Jan Kara wrote: > > > +int get_vaddr_frames(unsigned long start, unsigned int nr_frames, > > > + bool write, bool force, struct frame_vector *vec) > > > +{ > > > + struct mm_struct *mm = current->mm; > > > + struct vm_area_struct *vma; > > > + int ret = 0; > > > + int err; > > > + int locked = 1; > > > + > > > > bool locked. > It cannot be bool. It is passed to get_user_pages_locked() which expects > int *. > My bad. > > > +int frame_vector_to_pages(struct frame_vector *vec) > > > +{ > > > > I think it's probably best to make the relevant counters in frame_vector > > signed and limit the maximum possible size of it. It's still not putting > > any practical limit on the size of the frame_vector. > > I don't see a reason why counters in frame_vector should be signed... Can > you share your reason? I've added a check into frame_vector_create() to > limit number of frames to INT_MAX / sizeof(void *) / 2 to avoid arithmetics > overflow. Thanks for review! > Only that the return value of frame_vector_to_pages() returns int where as the potential range that is converted is unsigned int. I don't think there are any mistakes dealing with signed/unsigned but I don't see any advantage of using unsigned either and limiting it to INT_MAX either. It's not a big deal. -- Mel Gorman SUSE Labs -- 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>
next prev parent reply other threads:[~2015-05-11 14:47 UTC|newest] Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-05-06 7:28 [PATCH 0/9 v4] Helper to abstract vma handling in media layer Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` [PATCH 1/9] [media] vb2: Push mmap_sem down to memops Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` [PATCH 2/9] mm: Provide new get_vaddr_frames() helper Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 10:45 ` Vlastimil Babka 2015-05-06 10:45 ` Vlastimil Babka 2015-05-06 14:58 ` Jan Kara 2015-05-06 14:58 ` Jan Kara 2015-05-06 14:58 ` Jan Kara 2015-05-08 14:49 ` Mel Gorman 2015-05-08 14:49 ` Mel Gorman 2015-05-11 14:00 ` Jan Kara 2015-05-11 14:00 ` Jan Kara 2015-05-11 14:00 ` Jan Kara 2015-05-11 14:47 ` Mel Gorman [this message] 2015-05-11 14:47 ` Mel Gorman 2015-05-06 7:28 ` [PATCH 3/9] media: omap_vout: Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 10:46 ` Vlastimil Babka 2015-05-06 10:46 ` Vlastimil Babka 2015-05-06 15:01 ` Jan Kara 2015-05-06 15:01 ` Jan Kara 2015-05-06 7:28 ` [PATCH 4/9] vb2: Provide helpers for mapping virtual addresses Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` [PATCH 5/9] media: vb2: Convert vb2_dma_sg_get_userptr() to use frame vector Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` [PATCH 6/9] media: vb2: Convert vb2_vmalloc_get_userptr() " Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` [PATCH 7/9] media: vb2: Convert vb2_dc_get_userptr() " Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` [PATCH 8/9] media: vb2: Remove unused functions Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` [PATCH 9/9] drm/exynos: Convert g2d_userptr_get_dma_addr() to use get_vaddr_frames() Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 7:28 ` Jan Kara 2015-05-06 10:47 ` Vlastimil Babka 2015-05-06 10:47 ` Vlastimil Babka 2015-05-06 15:02 ` Jan Kara 2015-05-06 15:02 ` Jan Kara -- strict thread matches above, loose matches on Subject: below -- 2015-07-13 14:55 [PATCH 0/9 v7] Helper to abstract vma handling in media layer Jan Kara 2015-07-13 14:55 ` [PATCH 2/9] mm: Provide new get_vaddr_frames() helper Jan Kara 2015-07-13 14:55 ` Jan Kara 2015-07-17 10:23 ` Hans Verkuil 2015-07-17 10:23 ` Hans Verkuil 2015-07-17 20:26 ` Andrew Morton 2015-07-17 20:26 ` Andrew Morton 2015-05-13 13:08 [PATCH 0/9 v5] Helper to abstract vma handling in media layer Jan Kara 2015-05-13 13:08 ` [PATCH 2/9] mm: Provide new get_vaddr_frames() helper Jan Kara 2015-05-13 13:08 ` Jan Kara 2015-05-28 23:24 ` Andrew Morton 2015-05-28 23:24 ` Andrew Morton 2015-05-28 23:24 ` Andrew Morton 2015-06-01 12:40 ` Jan Kara 2015-06-01 12:40 ` Jan Kara 2015-06-01 12:40 ` Jan Kara 2015-06-01 13:02 ` Hans Verkuil 2015-06-01 13:02 ` Hans Verkuil 2015-06-02 15:23 ` Jan Kara 2015-06-02 15:23 ` Jan Kara 2015-06-02 15:23 ` Jan Kara 2015-06-02 22:29 ` Andrew Morton 2015-06-02 22:29 ` Andrew Morton 2015-06-03 9:34 ` Jan Kara 2015-06-03 9:34 ` Jan Kara 2015-05-05 16:01 [PATCH 0/9 v3] Helper to abstract vma handling in media layer Jan Kara 2015-05-05 16:01 ` [PATCH 2/9] mm: Provide new get_vaddr_frames() helper Jan Kara 2015-05-05 16:01 ` Jan Kara 2015-05-05 16:01 ` Jan Kara
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=20150511144707.GP2462@suse.de \ --to=mgorman@suse.de \ --cc=dri-devel@lists.freedesktop.org \ --cc=hverkuil@xs4all.nl \ --cc=jack@suse.cz \ --cc=linux-media@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=m.szyprowski@samsung.com \ --cc=mchehab@osg.samsung.com \ --cc=pawel@osciak.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.