Path: news.gmane.org!not-for-mail From: "Dan Williams" Newsgroups: gmane.linux.network,gmane.linux.kernel.announce Subject: Re: kmap_atomic() oopses in current mainline Date: Thu, 19 Jul 2007 08:23:02 -0700 Lines: 69 Approved: news@gmane.org Message-ID: References: <20070719013304.3c060c99.akpm@linux-foundation.org> <20070719092856.GA15839@2ka.mipt.ru> <20070719023831.400f7905.akpm@linux-foundation.org> <20070719100132.GC15839@2ka.mipt.ru> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Trace: sea.gmane.org 1184858621 19796 80.91.229.12 (19 Jul 2007 15:23:41 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Thu, 19 Jul 2007 15:23:41 +0000 (UTC) Cc: "Andrew Morton" , netdev@vger.kernel.org, linux-kernel-announce@vger.kernel.org To: "Evgeniy Polyakov" Original-X-From: netdev-owner@vger.kernel.org Thu Jul 19 17:23:39 2007 Return-path: Envelope-to: linux-netdev-2@gmane.org Original-Received: from vger.kernel.org ([209.132.176.167]) by lo.gmane.org with esmtp (Exim 4.50) id 1IBXqr-0007ZT-Ud for linux-netdev-2@gmane.org; Thu, 19 Jul 2007 17:23:38 +0200 Original-Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965032AbXGSPXL (ORCPT ); Thu, 19 Jul 2007 11:23:11 -0400 Original-Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1759901AbXGSPXK (ORCPT ); Thu, 19 Jul 2007 11:23:10 -0400 Original-Received: from an-out-0708.google.com ([209.85.132.242]:26495 "EHLO an-out-0708.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964979AbXGSPXF (ORCPT ); Thu, 19 Jul 2007 11:23:05 -0400 Original-Received: by an-out-0708.google.com with SMTP id d31so120347and for ; Thu, 19 Jul 2007 08:23:03 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth; b=dj2t8IG+TYsInD4tyZ38izTzTzvVLJRJ3sChdUn+8DKUXGuwmCYYzTu9CcAMF/MUTqqdjcY8ErSSct5gUrByfSkAIY5OCa9h0SDEptFkgBAib2NWHLkxVCeOiDRHkWyP+USJmgudek5szwE/NtyBAiyU5FSFrjUpm9q72NTEKF8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth; b=hIXdsPiVxDTI55GC0mX7odI3ceb/M68eZQFau1bkR+yVLCoyA4j1+pW2dJqWYqRskYJzFLtPcnVPBoe753X5nCe4+4/JBYxZp+Um0bw2IrIWt/w0sg2vYTeN9uE+iP7i7QiSOfuWGk3S5D7eWdtbtgQBKCQdwXXLUAVOZcpQGsc= Original-Received: by 10.100.10.20 with SMTP id 20mr958148anj.1184858582696; Thu, 19 Jul 2007 08:23:02 -0700 (PDT) Original-Received: by 10.100.109.17 with HTTP; Thu, 19 Jul 2007 08:23:01 -0700 (PDT) In-Reply-To: <20070719100132.GC15839@2ka.mipt.ru> Content-Disposition: inline X-Google-Sender-Auth: 12a2b05388f142ed Original-Sender: netdev-owner@vger.kernel.org Precedence: bulk X-Mailing-List: netdev@vger.kernel.org Xref: news.gmane.org gmane.linux.network:67027 gmane.linux.kernel.announce:462 Archived-At: On 7/19/07, Evgeniy Polyakov wrote: > On Thu, Jul 19, 2007 at 02:38:31AM -0700, Andrew Morton (akpm@linux-foundation.org) wrote: > > > > is very wrong if both ASYNC_TX_KMAP_DST and ASYNC_TX_KMAP_SRC can ever be > > > > set. We'll end up using the same kmap slot for both src add dest and we > > > > get either corrupted data or a BUG. > > > > > > So far it can not since the only user is raid code, which only allows to > > > perform either reading from bio or writing into one, which requires only > > > one mapping. > > > > hm, so we got lucky? > > I would say it was intentionally, current code can perform only one > operation in a time. Of course changing KM_USER from 0 to 1 in second > kmap_atomic will not force oceans to run out of coasts. > > Kind of: > > diff --git a/crypto/async_tx/async_memcpy.c b/crypto/async_tx/async_memcpy.c > index a973f4e..a48c7f3 100644 > --- a/crypto/async_tx/async_memcpy.c > +++ b/crypto/async_tx/async_memcpy.c > @@ -94,7 +94,7 @@ async_memcpy(struct page *dest, struct page *src, unsigned int dest_offset, > dest_buf = page_address(dest) + dest_offset; > > if (flags & ASYNC_TX_KMAP_SRC) > - src_buf = kmap_atomic(src, KM_USER0) + src_offset; > + src_buf = kmap_atomic(src, KM_USER1) + src_offset; > else > src_buf = page_address(src) + src_offset; > > @@ -104,7 +104,7 @@ async_memcpy(struct page *dest, struct page *src, unsigned int dest_offset, > kunmap_atomic(dest_buf, KM_USER0); > > if (flags & ASYNC_TX_KMAP_SRC) > - kunmap_atomic(src_buf, KM_USER0); > + kunmap_atomic(src_buf, KM_USER1); > > async_tx_sync_epilog(flags, depend_tx, cb_fn, cb_param); > } > > > > Btw, shouldn't it always be kmap_atomic() even if flag is not set. > > > That pages are usual one returned by alloc_page(). > > > > The code would work OK if the kmap_atomic()s were unconditional, but it > > would be a bit more expensive if the page is in highmem and we don't > > actually intend to access it with the CPU. > > > > kmap_atomic() against a non-highmem page is basically free: just an > > additional test_bit(). > Always kmap'ing the page is the way to go, since in this path the page is always accessed with the CPU. This also allows these ASYNC_TX_ flags to be killed off as they are not necessary. I'll cook up a patch, and be more careful about my kmap usage going forward. > As far as I recall there was an intention to do async memory copy to > userspace, so likely kmapping is a good idea. > > -- > Evgeniy Polyakov Thanks, Dan - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html