hi Kees Cook, On 2020/3/21 上午2:20, Kees Cook wrote: > 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. > How about rename blkzone.c to psotre_zone.c and blkoops.c to pstore_blk.c? Please refer to my reply email for patch 2. >>>>> 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. > -- WeiXiong Liao
