From mboxrd@z Thu Jan 1 00:00:00 1970 From: Haggai Eran Subject: Re: [PATCH for-next V1 2/5] IB/core: Add ib_is_udata_cleared Date: Sun, 13 Dec 2015 17:47:33 +0200 Message-ID: <566D9315.9050807@mellanox.com> References: <1449157463-25313-1-git-send-email-matanb@mellanox.com> <1449157463-25313-3-git-send-email-matanb@mellanox.com> <56658718.8080308@mellanox.com> <5669983E.5000200@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Matan Barak Cc: Matan Barak , Eli Cohen , linux-rdma , Doug Ledford , Eran Ben Elisha , Yann Droneaud List-Id: linux-rdma@vger.kernel.org On 10/12/2015 19:29, Matan Barak wrote: > On Thu, Dec 10, 2015 at 5:20 PM, Haggai Eran wrote: >> On 10/12/2015 16:59, Matan Barak wrote: >>> On Mon, Dec 7, 2015 at 3:18 PM, Haggai Eran wrote: >>>> On 12/03/2015 05:44 PM, Matan Barak wrote: >>>>> Extending core and vendor verb commands require us to check that the >>>>> unknown part of the user's given command is all zeros. >>>>> Adding ib_is_udata_cleared in order to do so. >>>>> >>>> >>>> Why not copy the data into kernel space and run memchr_inv() on it? >>>> >>> >>> Probably less efficient, isn't it? >> Why do you think it is less efficient? >> >> I'm not sure calling copy_from_user multiple times is very efficient. >> For once, you are calling access_ok multiple times. I guess it depends >> on the amount of data you are copying. >> > > Isn't access_ok pretty cheap? > It calls __chk_range_not_ok which on x86 seems like a very cheap > function and __chk_user_ptr which is a compiler check. > I guess most kernel-user implementation will be pretty much in sync, > so we'll possibly call it for a few/dozens of bytes. In that case, I > think this implementation is a bit faster. > >>> I know it isn't data path, but we'll execute this code in all extended >>> functions (sometimes even more than once). >> Do you think it is important enough to maintain our own copy of >> memchr_inv()? >> > > True, I'm not sure it's important enough, but do you think it's that > complicated? It is complicated in my opinion. It is 67 lines of code, it's architecture dependent and relies on preprocessor macros and conditional code. I think this kind of stuff belongs in lib/string.c and not in the RDMA stack. Haggai -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html