Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
From: Kees Cook <keescook@chromium.org>
To: WeiXiong Liao <liaoweixiong@allwinnertech.com>
Cc: Rob Herring <robh@kernel.org>, Tony Luck <tony.luck@intel.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Richard Weinberger <richard@nod.at>,
	Anton Vorontsov <anton@enomsg.org>,
	linux-doc@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Colin Cross <ccross@android.com>,
	linux-mtd@lists.infradead.org,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH v2 01/11] pstore/blk: new support logger for block devices
Date: Fri, 20 Mar 2020 11:20:22 -0700
Message-ID: <202003201111.BE5EAB9A@keescook> (raw)
In-Reply-To: <dab67ab1-c03f-0507-3d56-4a9578e85f6b@allwinnertech.com>

On Fri, Mar 20, 2020 at 09:50:36AM +0800, WeiXiong Liao wrote:
> On 2020/3/19 AM 1:23, Kees Cook wrote:
> > On Thu, Feb 27, 2020 at 04:21:51PM +0800, liaoweixiong wrote:
> >> On 2020/2/26 AM 8:52, Kees Cook wrote:
> >>> On Fri, Feb 07, 2020 at 08:25:45PM +0800, WeiXiong Liao wrote:
> >>>> +obj-$(CONFIG_PSTORE_BLK) += pstore_blk.o
> >>>> +pstore_blk-y += blkzone.o
> >>>
> >>> Why this dance with files? I would just expect:
> >>>
> >>> obj-$(CONFIG_PSTORE_BLK)     += blkzone.o
> >>>
> >>
> >> This makes the built module named blkzone.ko rather than
> >> pstore_blk.ko.
> > 
> > You can just do a regular build rule:
> > 
> > obj-$(CONFIG_PSTORE_BLK) += blkzone.o
> > 
> 
> I don't get it. If make it as your words, the built module will be
> blkzone.ko.
> The module is named pstore/blk, however it built out blkzone.ko. I think
> it's confusing.

I mean just pick whatever filename you want it to be named. The Makefile
case for ramoops was that ramoops got renamed but we wanted to keep the
old API name.

So, if you want it named pstore-blk.ko, just rename blkzone.c to
pstore-blk.c.

> >>> If you're expecting concurrent writers (use of atomic_set(), I would
> >>> expect the whole write to be locked instead. (i.e. what happens if
> >>> multiple callers call blkz_zone_write()?)
> >>>
> >>
> >> I don't agree with it. The datalen will be updated everywhere. It's useless
> >> to lock here.
> > 
> > But there could be multiple writers; locking should be needed.
> > 
> 
> All the recorders such as dmesg, pmsg, console and ftrace have been
> locked on
> pstore and upper layers. So, a recorder will not write in parallel and
> different
> recorders operate privately zone. They don't have any influence on each
> other.

Yes, sorry, I was confusing myself about pmsg, and I forgot it had a
global lock. Each are locked or split by CPU.

> The only parallel case I think is that recorder writes while dirty-flush
> thread is
> working. And the dirty-flusher will flush the whole zone rather than
> part of it, so,
> it is OK to call in parallel.

Okay, thanks for clarifying.

> Based on these reasons, I don't think locking should be needed.

Agreed.

-- 
Kees Cook

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply index

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-07 12:25 [PATCH v2 00/11] pstore: mtd: support crash log to block and mtd device WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 01/11] pstore/blk: new support logger for block devices WeiXiong Liao
2020-02-26  0:52   ` Kees Cook
2020-02-27  8:21     ` liaoweixiong
2020-03-18 17:23       ` Kees Cook
2020-03-20  1:50         ` WeiXiong Liao
2020-03-20 18:20           ` Kees Cook [this message]
2020-03-22 10:28             ` WeiXiong Liao
2020-03-09  0:52     ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 02/11] blkoops: add blkoops, a warpper for pstore/blk WeiXiong Liao
2020-03-18 18:06   ` Kees Cook
2020-03-22 10:00     ` WeiXiong Liao
2020-03-22 15:44       ` Kees Cook
2020-02-07 12:25 ` [PATCH v2 03/11] pstore/blk: blkoops: support pmsg recorder WeiXiong Liao
2020-03-18 18:13   ` Kees Cook
2020-03-22 11:14     ` WeiXiong Liao
2020-03-22 15:59       ` Kees Cook
2020-02-07 12:25 ` [PATCH v2 04/11] pstore/blk: blkoops: support console recorder WeiXiong Liao
2020-03-18 18:16   ` Kees Cook
2020-03-22 11:35     ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 05/11] pstore/blk: blkoops: support ftrace recorder WeiXiong Liao
2020-03-18 18:19   ` Kees Cook
2020-03-22 11:42     ` WeiXiong Liao
2020-03-22 15:16       ` Kees Cook
2020-02-07 12:25 ` [PATCH v2 06/11] Documentation: pstore/blk: blkoops: create document for pstore_blk WeiXiong Liao
2020-03-18 18:31   ` Kees Cook
2020-03-22 12:20     ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 07/11] pstore/blk: skip broken zone for mtd device WeiXiong Liao
2020-03-18 18:35   ` Kees Cook
2020-03-22 12:27     ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 08/11] blkoops: respect for device to pick recorders WeiXiong Liao
2020-03-18 18:42   ` Kees Cook
2020-03-22 13:06     ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 09/11] pstore/blk: blkoops: support special removing jobs for dmesg WeiXiong Liao
2020-03-18 18:47   ` Kees Cook
2020-03-22 13:03     ` WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 10/11] blkoops: add interface for dirver to get information of blkoops WeiXiong Liao
2020-02-07 12:25 ` [PATCH v2 11/11] mtd: new support oops logger based on pstore/blk WeiXiong Liao
2020-02-18 10:34   ` Miquel Raynal
2020-02-19  1:13     ` liaoweixiong
2020-03-18 18:57   ` Kees Cook
2020-03-22 13:51     ` WeiXiong Liao
2020-03-22 15:13       ` Kees Cook

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=202003201111.BE5EAB9A@keescook \
    --to=keescook@chromium.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=anton@enomsg.org \
    --cc=ccross@android.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=liaoweixiong@allwinnertech.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=mchehab+samsung@kernel.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=robh@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=vigneshr@ti.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

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git