* Re: [PATCH v3 09/11] memcg: add cgroupfs interface to memcg dirty limits
From: Daisuke Nishimura @ 2010-10-20 3:31 UTC (permalink / raw)
To: Greg Thelen
Cc: Andrew Morton, linux-kernel, linux-mm, containers, Andrea Righi,
Balbir Singh, KAMEZAWA Hiroyuki, Minchan Kim, Ciju Rajan K,
David Rientjes, Daisuke Nishimura
In-Reply-To: <1287448784-25684-10-git-send-email-gthelen@google.com>
On Mon, 18 Oct 2010 17:39:42 -0700
Greg Thelen <gthelen@google.com> wrote:
> Add cgroupfs interface to memcg dirty page limits:
> Direct write-out is controlled with:
> - memory.dirty_ratio
> - memory.dirty_limit_in_bytes
>
> Background write-out is controlled with:
> - memory.dirty_background_ratio
> - memory.dirty_background_limit_bytes
>
> Other memcg cgroupfs files support 'M', 'm', 'k', 'K', 'g'
> and 'G' suffixes for byte counts. This patch provides the
> same functionality for memory.dirty_limit_in_bytes and
> memory.dirty_background_limit_bytes.
>
> Signed-off-by: Andrea Righi <arighi@develer.com>
> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> Signed-off-by: Greg Thelen <gthelen@google.com>
Acked-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
One question: shouldn't we return -EINVAL when writing to dirty(_background)_limit_bytes
a bigger value than that of global one(if any) ? Or do you intentionally
set the input value without comparing it with the global value ?
But, hmm..., IMHO we should check it in __mem_cgroup_dirty_param() or something
not to allow dirty pages more than global limit.
Thanks,
Daisuke Nishimura.
^ permalink raw reply
* Re: read/write on RADOS using external buffer
From: Colin McCabe @ 2010-10-20 3:35 UTC (permalink / raw)
To: Takuya ASADA; +Cc: Sage Weil, ceph-devel
In-Reply-To: <20101019222727.GA1737@enoki.dokukino.com>
Hi Takuya,
Thanks for looking at this!
Unfortunately I think you have taken the wrong approach here. There
isn't any need for thread-local data in the Objecter. It's not threads
that the Objecter cares about, it's clients. Also, I don't think a
change like this needs to involve SimpleMessenger and all that.
Take a look at rados_read in librados.cc. This function implements the
C API, which can already handle using external buffers. It may be that
all you need to do is add functions in the C++ header that call
rados_read and rados_write!
I actually think that rados_read could be made slightly more
efficient. We could probably add a bufferlist constructor that starts
with an already allocated buffer, to avoid the call to bl.copy. I
haven't thought too hard about how to do this but in principle it
seems reasonable.
regards,
Colin McCabe
On Tue, Oct 19, 2010 at 3:27 PM, Takuya ASADA <syuu@dokukino.com> wrote:
> Hi,
>
> I just implemented the patch to support external buffer on Rados::read() and Rados::write().
>
> diff --git a/src/include/librados.hpp b/src/include/librados.hpp
> index 06fa3b2..bfb0f5b 100644
> --- a/src/include/librados.hpp
> +++ b/src/include/librados.hpp
> @@ -63,8 +63,10 @@ public:
> int create(pool_t pool, const std::string& oid, bool exclusive);
>
> int write(pool_t pool, const std::string& oid, off_t off, bufferlist& bl, size_t len);
> + int write(pool_t pool, const std::string& oid, off_t off, void *buf, size_t len);
> int write_full(pool_t pool, const std::string& oid, bufferlist& bl);
> int read(pool_t pool, const std::string& oid, off_t off, bufferlist& bl, size_t len);
> + int read(pool_t pool, const std::string& oid, off_t off, void *buf, size_t len);
> int remove(pool_t pool, const std::string& oid);
> int trunc(pool_t pool, const std::string& oid, size_t size);
>
> @@ -135,4 +137,3 @@ public:
> }
>
> #endif
> -
> diff --git a/src/librados.cc b/src/librados.cc
> index 4c8a464..91e6a27 100644
> --- a/src/librados.cc
> +++ b/src/librados.cc
> @@ -72,11 +72,12 @@ class RadosClient : public Dispatcher
>
> Mutex lock;
> Cond cond;
> + static hash_map<tid_t, bufferptr*> buffer_map;
> + static bufferptr* fetch_buffer_func(tid_t tid);
>
> -
> public:
> RadosClient() : messenger(NULL), lock("radosclient") {
> - messenger = new SimpleMessenger();
> + messenger = new SimpleMessenger(&RadosClient::fetch_buffer_func);
> }
>
> ~RadosClient();
> @@ -132,8 +133,10 @@ public:
> // io
> int create(PoolCtx& pool, const object_t& oid, bool exclusive);
> int write(PoolCtx& pool, const object_t& oid, off_t off, bufferlist& bl, size_t len);
> + int write(PoolCtx& pool, const object_t& oid, off_t off, void *buf, size_t len);
> int write_full(PoolCtx& pool, const object_t& oid, bufferlist& bl);
> int read(PoolCtx& pool, const object_t& oid, off_t off, bufferlist& bl, size_t len);
> + int read(PoolCtx& pool, const object_t& oid, off_t off, void *buf, size_t len);
> int remove(PoolCtx& pool, const object_t& oid);
> int stat(PoolCtx& pool, const object_t& oid, uint64_t *psize, time_t *pmtime);
> int trunc(PoolCtx& pool, const object_t& oid, size_t size);
> @@ -870,6 +873,15 @@ int RadosClient::write(PoolCtx& pool, const object_t& oid, off_t off, bufferlist
> return len;
> }
>
> +int RadosClient::write(PoolCtx& pool, const object_t& oid, off_t off, void *buf, size_t len)
> +{
> + bufferptr bp = buffer::create_static(len, static_cast<char *>(buf));
> + bufferlist bl;
> +
> + bl.push_back(bp);
> + return write(pool, oid, off, bl, len);
> +}
> +
> int RadosClient::write_full(PoolCtx& pool, const object_t& oid, bufferlist& bl)
> {
> utime_t ut = g_clock.now();
> @@ -1116,6 +1128,46 @@ int RadosClient::read(PoolCtx& pool, const object_t& oid, off_t off, bufferlist&
> return bl.length();
> }
>
> +int RadosClient::read(PoolCtx& pool, const object_t& oid, off_t off, void *buf, size_t len)
> +{
> + SnapContext snapc;
> +
> + Mutex mylock("RadosClient::read::mylock");
> + Cond cond;
> + bool done;
> + int r;
> + Context *onack = new C_SafeCond(&mylock, &cond, &done, &r);
> + bufferptr bp = buffer::create_static(len, static_cast<char *>(buf));
> + bufferlist bl;
> +
> + bl.push_back(bp);
> + lock.Lock();
> + ceph_object_layout layout = objecter->osdmap->make_object_layout(oid, pool.poolid);
> + tid_t tid = objecter->get_tid();
> + buffer_map[tid] = &bp;
> + objecter->read_with_tid(oid, layout,
> + off, len, pool.snap_seq, &bl, 0,
> + onack, tid);
> + lock.Unlock();
> +
> + mylock.Lock();
> + while (!done)
> + cond.Wait(mylock);
> + mylock.Unlock();
> + buffer_map.erase(tid);
> + dout(10) << "Objecter returned from read r=" << r << dendl;
> +
> + if (r < 0)
> + return r;
> +
> + if (bl.length() < len) {
> + dout(10) << "Returned length " << bl.length()
> + << " less than original length "<< len << dendl;
> + }
> +
> + return bl.length();
> +}
> +
> int RadosClient::stat(PoolCtx& pool, const object_t& oid, uint64_t *psize, time_t *pmtime)
> {
> SnapContext snapc;
> @@ -1251,6 +1303,13 @@ int RadosClient::getxattrs(PoolCtx& pool, const object_t& oid, map<std::string,
> return r;
> }
>
> +hash_map<tid_t, bufferptr*> RadosClient::buffer_map;
> +
> +bufferptr* RadosClient::fetch_buffer_func(tid_t tid)
> +{
> + return buffer_map[tid];
> +}
> +
> // ---------------------------------------------
>
> namespace librados {
> @@ -1401,6 +1460,14 @@ int Rados::write(rados_pool_t pool, const string& o, off_t off, bufferlist& bl,
> return ((RadosClient *)client)->write(*(RadosClient::PoolCtx *)pool, oid, off, bl, len);
> }
>
> +int Rados::write(rados_pool_t pool, const string& o, off_t off, void *buf, size_t len)
> +{
> + if (!client)
> + return -EINVAL;
> + object_t oid(o);
> + return ((RadosClient *)client)->write(*(RadosClient::PoolCtx *)pool, oid, off, buf, len);
> +}
> +
> int Rados::write_full(rados_pool_t pool, const string& o, bufferlist& bl)
> {
> if (!client)
> @@ -1433,6 +1500,14 @@ int Rados::read(rados_pool_t pool, const string& o, off_t off, bufferlist& bl, s
> return ((RadosClient *)client)->read(*(RadosClient::PoolCtx *)pool, oid, off, bl, len);
> }
>
> +int Rados::read(rados_pool_t pool, const string& o, off_t off, void *buf, size_t len)
> +{
> + if (!client)
> + return -EINVAL;
> + object_t oid(o);
> + return ((RadosClient *)client)->read(*(RadosClient::PoolCtx *)pool, oid, off, buf, len);
> +}
> +
> int Rados::getxattr(rados_pool_t pool, const string& o, const char *name, bufferlist& bl)
> {
> if (!client)
> diff --git a/src/msg/SimpleMessenger.cc b/src/msg/SimpleMessenger.cc
> index 4632267..75b4576 100644
> --- a/src/msg/SimpleMessenger.cc
> +++ b/src/msg/SimpleMessenger.cc
> @@ -51,7 +51,7 @@ static ostream& _prefix(SimpleMessenger *messenger) {
> #define closed_socket() //dout(20) << "closed_socket " << --sockopen << dendl;
> #define opened_socket() //dout(20) << "opened_socket " << ++sockopen << dendl;
>
> -
> +SimpleMessenger::fetch_buffer_callback_t SimpleMessenger::fetch_buffer_callback = 0;
>
> /********************************************
> * Accepter
> @@ -1786,36 +1786,47 @@ int SimpleMessenger::Pipe::read_message(Message **pm)
> data_len = le32_to_cpu(header.data_len);
> data_off = le32_to_cpu(header.data_off);
> if (data_len) {
> - int left = data_len;
> - if (data_off & ~PAGE_MASK) {
> - // head
> - int head = MIN(PAGE_SIZE - (data_off & ~PAGE_MASK),
> - (unsigned)left);
> - bufferptr bp = buffer::create(head);
> - if (tcp_read( sd, bp.c_str(), head, messenger->timeout ) < 0)
> + if (fetch_buffer_callback) {
> + bufferptr *bpp = fetch_buffer_callback(header.tid);
> + if (!bpp)
> + goto allocate_buffer;
> + if (tcp_read( sd, bpp->c_str(), data_len, messenger->timeout ) < 0)
> goto out_dethrottle;
> - data.push_back(bp);
> - left -= head;
> - dout(20) << "reader got data head " << head << dendl;
> - }
> + data.push_back(*bpp);
> + dout(20) << "reader got data " << data_len << dendl;
> + }else{
> + allocate_buffer:
> + int left = data_len;
> + if (data_off & ~PAGE_MASK) {
> + // head
> + int head = MIN(PAGE_SIZE - (data_off & ~PAGE_MASK),
> + (unsigned)left);
> + bufferptr bp = buffer::create(head);
> + if (tcp_read( sd, bp.c_str(), head, messenger->timeout ) < 0)
> + goto out_dethrottle;
> + data.push_back(bp);
> + left -= head;
> + dout(20) << "reader got data head " << head << dendl;
> + }
>
> - // middle
> - int middle = left & PAGE_MASK;
> - if (middle > 0) {
> - bufferptr bp = buffer::create_page_aligned(middle);
> - if (tcp_read( sd, bp.c_str(), middle, messenger->timeout ) < 0)
> - goto out_dethrottle;
> - data.push_back(bp);
> - left -= middle;
> - dout(20) << "reader got data page-aligned middle " << middle << dendl;
> - }
> + // middle
> + int middle = left & PAGE_MASK;
> + if (middle > 0) {
> + bufferptr bp = buffer::create_page_aligned(middle);
> + if (tcp_read( sd, bp.c_str(), middle, messenger->timeout ) < 0)
> + goto out_dethrottle;
> + data.push_back(bp);
> + left -= middle;
> + dout(20) << "reader got data page-aligned middle " << middle << dendl;
> + }
>
> - if (left) {
> - bufferptr bp = buffer::create(left);
> - if (tcp_read( sd, bp.c_str(), left, messenger->timeout ) < 0)
> - goto out_dethrottle;
> - data.push_back(bp);
> - dout(20) << "reader got data tail " << left << dendl;
> + if (left) {
> + bufferptr bp = buffer::create(left);
> + if (tcp_read( sd, bp.c_str(), left, messenger->timeout ) < 0)
> + goto out_dethrottle;
> + data.push_back(bp);
> + dout(20) << "reader got data tail " << left << dendl;
> + }
> }
> }
>
> diff --git a/src/msg/SimpleMessenger.h b/src/msg/SimpleMessenger.h
> index b4a0ef3..72a5a1b 100644
> --- a/src/msg/SimpleMessenger.h
> +++ b/src/msg/SimpleMessenger.h
> @@ -567,6 +567,9 @@ private:
> SimpleMessenger *messenger; //hack to make dout macro work, will fix
> int timeout;
>
> + typedef bufferptr* (*fetch_buffer_callback_t) (tid_t);
> + static fetch_buffer_callback_t fetch_buffer_callback;
> +
> public:
> SimpleMessenger() :
> Messenger(entity_name_t()),
> @@ -580,6 +583,20 @@ public:
> // for local dmsg delivery
> dispatch_queue.local_pipe = new Pipe(this, Pipe::STATE_OPEN);
> }
> +
> + SimpleMessenger(fetch_buffer_callback_t callback) :
> + Messenger(entity_name_t()),
> + accepter(this),
> + lock("SimpleMessenger::lock"), started(false), did_bind(false),
> + dispatch_throttler(g_conf.ms_dispatch_throttle_bytes), need_addr(true),
> + destination_stopped(true), my_type(-1),
> + global_seq_lock("SimpleMessenger::global_seq_lock"), global_seq(0),
> + reaper_thread(this), reaper_started(false), reaper_stop(false),
> + dispatch_thread(this), messenger(this) {
> + fetch_buffer_callback = callback;
> + // for local dmsg delivery
> + dispatch_queue.local_pipe = new Pipe(this, Pipe::STATE_OPEN);
> + }
> ~SimpleMessenger() {
> delete dispatch_queue.local_pipe;
> }
> diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
> index a34c0a9..fd43795 100644
> --- a/src/osdc/Objecter.h
> +++ b/src/osdc/Objecter.h
> @@ -530,6 +530,21 @@ private:
> o->outbl = pbl;
> return op_submit(o);
> }
> + tid_t read_with_tid(const object_t& oid, ceph_object_layout ol,
> + uint64_t off, uint64_t len, snapid_t snap, bufferlist *pbl, int flags,
> + Context *onfinish, tid_t tid) {
> + vector<OSDOp> ops(1);
> + ops[0].op.op = CEPH_OSD_OP_READ;
> + ops[0].op.extent.offset = off;
> + ops[0].op.extent.length = len;
> + ops[0].op.extent.truncate_size = 0;
> + ops[0].op.extent.truncate_seq = 0;
> + Op *o = new Op(oid, ol, ops, flags, onfinish, 0);
> + o->snapid = snap;
> + o->outbl = pbl;
> + o->tid = tid;
> + return op_submit(o);
> + }
> tid_t read_trunc(const object_t& oid, ceph_object_layout ol,
> uint64_t off, uint64_t len, snapid_t snap, bufferlist *pbl, int flags,
> uint64_t trunc_size, __u32 trunc_seq,
> @@ -725,6 +740,8 @@ private:
>
> void list_objects(ListContext *p, Context *onfinish);
>
> + tid_t get_tid(void) { return ++last_tid; }
> +
> // -------------------------
> // pool ops
> private:
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: pl2303 ttyUSB0: pl2303_write - failed submitting write urb, error -22
From: Raju Rameshwar Uprade @ 2010-10-20 3:34 UTC (permalink / raw)
To: Greg KH, Raju Rameshwar Uprade; +Cc: linux-kernel, linux-usb
In-Reply-To: <20101019124713.GA31828@kroah.com>
> Then you are really on your own here, sorry.
Greg,
I have learned a lot from you..I am very close to the solution...thanks for your
wishes..I think in coming weeks, I will be able to resolve the issues.
I will always be very thankful for your support and guidance.
Regards,
Raj.
^ permalink raw reply
* Re: [PATCH] drivers/hwmon: Use pr_fmt and pr_<level>
From: Joe Perches @ 2010-10-20 3:34 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean Delvare, Hans de Goede, Alistair John Strachan,
Henrik Rydberg, Mark M. Hoffman, Luca Tettamanti, Fenghua Yu,
Juerg Haefliger, Eric Piel, Jim Cromie, Roger Lucas,
lm-sensors@lm-sensors.org, LKML
In-Reply-To: <20101020032938.GA20095@ericsson.com>
On Tue, 2010-10-19 at 20:29 -0700, Guenter Roeck wrote:
> On Tue, Oct 19, 2010 at 07:13:40PM -0400, Joe Perches wrote:
> > Convert printks to pr_<level>
> > Coalesce long formats
> > Removed prefixes from formats
> > Added #define pr_fmt KBUILD_MODNAME ": " fmt
> > Standardized abitguru messages for reporting and finding MAINTAINER
> > Compiled x86 allyesconfig only.
> > I inspected "strings drivers/hwmon/built-in.o",
> > but it's otherwise untested.
> > Signed-off-by: Joe Perches <joe@perches.com>
> There are several lines longer than 80 characters.
> Does this rule no longer apply ?
80 columns isn't checked for printk format strings.
A kernel general preference may be to keep formats as
a single string without line breaks so that grep works
better.
> Oddly enough, there are only four checkpatch warnings about long lines,
> even though there are many more.
The version I use doesn't show any warnings.
cheers, Joe
^ permalink raw reply
* Re: Issues with parallel build? (Was: gtk+native missing dep ?)
From: Denys Dmytriyenko @ 2010-10-20 3:33 UTC (permalink / raw)
To: openembedded-devel
In-Reply-To: <AANLkTimze8vRUUloa6gaRBU6jvp8zrcSAKDvqCu3ppaY@mail.gmail.com>
On Thu, May 20, 2010 at 12:57:23AM +0200, Andrea Adami wrote:
> Same issue happens during rebuild from packaged staging
>
>
> Cannot load module
> /oe/build/tmp/work/i686-linux/gtk+-native-2.20.0-r8.0/gtk+-2.20.0/modules/input/im-xim.la:
> /oe/build/tmp/work/i686-linux/gtk+-native-2.20.0-r8.0/gtk+-2.20.0/modules/input/.libs/im-xim.so:
> undefined symbol: g_realloc_n
> | /oe/build/tmp/work/i686-linux/gtk+-native-2.20.0-r8.0/gtk+-2.20.0/modules/input/im-xim.la
> does not export GTK+ IM module API:
> /oe/build/tmp/work/i686-linux/gtk+-native-2.20.0-r8.0/gtk+-2.20.0/modules/input/.libs/im-xim.so:
> undefined symbol: g_realloc_n
If anybody cares, this issue is still present with the latest glib/gtk+ on
Angstrom with 4+ threads...
--
Denys
>
> ...
>
> NOTE: package gtk+-native-2.20.0-r8.0: task do_compile: Failed
> ERROR: TaskFailed event exception, aborting
> ERROR: Build of
> virtual:native:/oe/openembedded/recipes/gtk+/gtk+_2.20.0.bb do_compile
> failed
> ...
>
> Andrea
^ permalink raw reply
* Re: [lm-sensors] [PATCH] drivers/hwmon: Use pr_fmt and pr_<level>
From: Joe Perches @ 2010-10-20 3:34 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jean Delvare, Hans de Goede, Alistair John Strachan,
Henrik Rydberg, Mark M. Hoffman, Luca Tettamanti, Fenghua Yu,
Juerg Haefliger, Eric Piel, Jim Cromie, Roger Lucas,
lm-sensors@lm-sensors.org, LKML
In-Reply-To: <20101020032938.GA20095@ericsson.com>
On Tue, 2010-10-19 at 20:29 -0700, Guenter Roeck wrote:
> On Tue, Oct 19, 2010 at 07:13:40PM -0400, Joe Perches wrote:
> > Convert printks to pr_<level>
> > Coalesce long formats
> > Removed prefixes from formats
> > Added #define pr_fmt KBUILD_MODNAME ": " fmt
> > Standardized abitguru messages for reporting and finding MAINTAINER
> > Compiled x86 allyesconfig only.
> > I inspected "strings drivers/hwmon/built-in.o",
> > but it's otherwise untested.
> > Signed-off-by: Joe Perches <joe@perches.com>
> There are several lines longer than 80 characters.
> Does this rule no longer apply ?
80 columns isn't checked for printk format strings.
A kernel general preference may be to keep formats as
a single string without line breaks so that grep works
better.
> Oddly enough, there are only four checkpatch warnings about long lines,
> even though there are many more.
The version I use doesn't show any warnings.
cheers, Joe
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply
* Re: [PATCH v3 09/11] memcg: add cgroupfs interface to memcg dirty limits
From: Daisuke Nishimura @ 2010-10-20 3:31 UTC (permalink / raw)
To: Greg Thelen
Cc: Andrew Morton, linux-kernel, linux-mm, containers, Andrea Righi,
Balbir Singh, KAMEZAWA Hiroyuki, Minchan Kim, Ciju Rajan K,
David Rientjes, Daisuke Nishimura
In-Reply-To: <1287448784-25684-10-git-send-email-gthelen@google.com>
On Mon, 18 Oct 2010 17:39:42 -0700
Greg Thelen <gthelen@google.com> wrote:
> Add cgroupfs interface to memcg dirty page limits:
> Direct write-out is controlled with:
> - memory.dirty_ratio
> - memory.dirty_limit_in_bytes
>
> Background write-out is controlled with:
> - memory.dirty_background_ratio
> - memory.dirty_background_limit_bytes
>
> Other memcg cgroupfs files support 'M', 'm', 'k', 'K', 'g'
> and 'G' suffixes for byte counts. This patch provides the
> same functionality for memory.dirty_limit_in_bytes and
> memory.dirty_background_limit_bytes.
>
> Signed-off-by: Andrea Righi <arighi@develer.com>
> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> Signed-off-by: Greg Thelen <gthelen@google.com>
Acked-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
One question: shouldn't we return -EINVAL when writing to dirty(_background)_limit_bytes
a bigger value than that of global one(if any) ? Or do you intentionally
set the input value without comparing it with the global value ?
But, hmm..., IMHO we should check it in __mem_cgroup_dirty_param() or something
not to allow dirty pages more than global limit.
Thanks,
Daisuke Nishimura.
--
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>
^ permalink raw reply
* Re: [PATCH] drivers/hwmon: Use pr_fmt and pr_<level>
From: Guenter Roeck @ 2010-10-20 3:29 UTC (permalink / raw)
To: Joe Perches
Cc: Jean Delvare, Hans de Goede, Alistair John Strachan,
Henrik Rydberg, Mark M. Hoffman, Luca Tettamanti, Fenghua Yu,
Juerg Haefliger, Eric Piel, Jim Cromie, Roger Lucas,
lm-sensors@lm-sensors.org, LKML
In-Reply-To: <1287530020.10409.594.camel@Joe-Laptop>
On Tue, Oct 19, 2010 at 07:13:40PM -0400, Joe Perches wrote:
> Convert printks to pr_<level>
> Coalesce long formats
> Removed prefixes from formats
> Added #define pr_fmt KBUILD_MODNAME ": " fmt
> Standardized abitguru messages for reporting and finding MAINTAINER
>
> Compiled x86 allyesconfig only.
> I inspected "strings drivers/hwmon/built-in.o",
> but it's otherwise untested.
>
> Signed-off-by: Joe Perches <joe@perches.com>
There are several lines longer than 80 characters.
Does this rule no longer apply ?
Oddly enough, there are only four checkpatch warnings about long lines,
even though there are many more.
Guenter
^ permalink raw reply
* Re: [patch 31/35] fs: icache per-zone inode LRU
From: KOSAKI Motohiro @ 2010-10-20 3:29 UTC (permalink / raw)
To: Nick Piggin; +Cc: kosaki.motohiro, Dave Chinner, linux-kernel, linux-fsdevel
In-Reply-To: <20101020032024.GA4134@amd>
> On Wed, Oct 20, 2010 at 12:14:32PM +0900, KOSAKI Motohiro wrote:
> > > On Tue, Oct 19, 2010 at 02:42:47PM +1100, npiggin@kernel.dk wrote:
> > > Anyway, my main point is that tying the LRU and shrinker scaling to
> > > the implementation of the VM is a one-off solution that doesn't work
> > > for generic infrastructure. Other subsystems need the same
> > > large-machine scaling treatment, and there's no way we should be
> > > tying them all into the struct zone. It needs further abstraction.
> >
> > I'm not sure what data structure is best. I can only say current
> > zone unawareness slab shrinker might makes following sad scenario.
> >
> > o DMA zone shortage invoke and plenty icache in NORMAL zone dropping
> > o NUMA aware system enable zone_reclaim_mode, but shrink_slab() still
> > drop unrelated zone's icache
> >
> > both makes performance degression. In other words, Linux does not have
> > flat memory model. so, I don't think Nick's basic concept is wrong.
> > It's straight forward enhancement. but if it don't fit current shrinkers,
> > I'd like to discuss how to make better data structure.
> >
> >
> >
> > and I have dump question (sorry, I don't know xfs at all). current
> > xfs_mount is below.
> >
> > typedef struct xfs_mount {
> > ...
> > struct shrinker m_inode_shrink; /* inode reclaim shrinker */
> > } xfs_mount_t;
> >
> >
> > Do you mean xfs can't convert shrinker to shrinker[ZONES]? If so, why?
>
> Well if XFS were to use per-ZONE shrinkers, it would remain with a
> single shrinker context per-sb like it has now, but it would divide
> its object management into per-zone structures.
Oops, my fault ;)
Yes, my intention is converting mp->m_perag_tree to per-zone.
Thanks fix me.
>
> For subsystems that aren't important, don't take much memory or have
> much reclaim throughput, they are free to ignore the zone argument
> and keep using the global input to the shrinker.
>
^ permalink raw reply
* Re: [lm-sensors] [PATCH] drivers/hwmon: Use pr_fmt and pr_<level>
From: Guenter Roeck @ 2010-10-20 3:29 UTC (permalink / raw)
To: Joe Perches
Cc: Jean Delvare, Hans de Goede, Alistair John Strachan,
Henrik Rydberg, Mark M. Hoffman, Luca Tettamanti, Fenghua Yu,
Juerg Haefliger, Eric Piel, Jim Cromie, Roger Lucas,
lm-sensors@lm-sensors.org, LKML
In-Reply-To: <1287530020.10409.594.camel@Joe-Laptop>
On Tue, Oct 19, 2010 at 07:13:40PM -0400, Joe Perches wrote:
> Convert printks to pr_<level>
> Coalesce long formats
> Removed prefixes from formats
> Added #define pr_fmt KBUILD_MODNAME ": " fmt
> Standardized abitguru messages for reporting and finding MAINTAINER
>
> Compiled x86 allyesconfig only.
> I inspected "strings drivers/hwmon/built-in.o",
> but it's otherwise untested.
>
> Signed-off-by: Joe Perches <joe@perches.com>
There are several lines longer than 80 characters.
Does this rule no longer apply ?
Oddly enough, there are only four checkpatch warnings about long lines,
even though there are many more.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply
* Re: [PATCH] net: make ctl_path local and const
From: Joe Perches @ 2010-10-20 3:28 UTC (permalink / raw)
To: Changli Gao
Cc: Andy Grover, James Morris, linux-sctp, rds-devel,
Pekka Savola (ipv6), linux-x25, dccp, bridge, Andrew, coreteam,
Arnaldo Carvalho de Melo, Alexey Kuznetsov, Joerg Reuter,
Sridhar Samudrala, Samuel Ortiz, Vlad Yasevich, netfilter,
Remi Denis-Courmont, linux-hams, Hideaki YOSHIFUJI, netdev,
linux-decnet-user, linux-kernel
In-Reply-To: <AANLkTi=pz4zYnnUSvb9FjVvCAURruHhJTz7pBKDi8Pdw@mail.gmail.com>
On Wed, 2010-10-20 at 11:10 +0800, Changli Gao wrote:
> On Wed, Oct 20, 2010 at 11:01 AM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2010-10-20 at 10:54 +0800, Changli Gao wrote:
> >> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> > []
> >> diff --git a/net/appletalk/sysctl_net_atalk.c b/net/appletalk/sysctl_net_atalk.c
> >> index 04e9c0d..b92f269 100644
> >> --- a/net/appletalk/sysctl_net_atalk.c
> >> +++ b/net/appletalk/sysctl_net_atalk.c
> >> @@ -42,16 +42,16 @@ static struct ctl_table atalk_table[] = {
> >> { },
> >> };
> >> -static struct ctl_path atalk_path[] = {
> >> - { .procname = "net", },
> >> - { .procname = "appletalk", },
> >> - { }
> >> -};
> >> -
> >> static struct ctl_table_header *atalk_table_header;
> >>
> >> void atalk_register_sysctl(void)
> >> {
> >> + const struct ctl_path atalk_path[] = {
> > Shouldn't all of these be static const struct ?
> They needn't. And some variables are specified __net_initdata currently.
At least some objects are smaller with static.
$ size net/appletalk/sysctl_net_atalk.o.*
text data bss dec hex filename
324 236 48 608 260 net/appletalk/sysctl_net_atalk.o.withstatic
344 236 48 628 274 net/appletalk/sysctl_net_atalk.o.withoutstatic
^ permalink raw reply
* Re: [PATCH] net: make ctl_path local and const
From: Joe Perches @ 2010-10-20 3:28 UTC (permalink / raw)
To: Changli Gao
Cc: Andy Grover, James Morris, linux-sctp, rds-devel,
Pekka Savola (ipv6), linux-x25, dccp, bridge, Andrew, coreteam,
Arnaldo Carvalho de Melo, Alexey Kuznetsov, Joerg Reuter,
Sridhar Samudrala, Samuel Ortiz, Vlad Yasevich, netfilter,
Remi Denis-Courmont, linux-hams, Hideaki YOSHIFUJI, netdev,
linux-decnet-user, linux-kernel
In-Reply-To: <AANLkTi=pz4zYnnUSvb9FjVvCAURruHhJTz7pBKDi8Pdw@mail.gmail.com>
On Wed, 2010-10-20 at 11:10 +0800, Changli Gao wrote:
> On Wed, Oct 20, 2010 at 11:01 AM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2010-10-20 at 10:54 +0800, Changli Gao wrote:
> >> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> > []
> >> diff --git a/net/appletalk/sysctl_net_atalk.c b/net/appletalk/sysctl_net_atalk.c
> >> index 04e9c0d..b92f269 100644
> >> --- a/net/appletalk/sysctl_net_atalk.c
> >> +++ b/net/appletalk/sysctl_net_atalk.c
> >> @@ -42,16 +42,16 @@ static struct ctl_table atalk_table[] = {
> >> { },
> >> };
> >> -static struct ctl_path atalk_path[] = {
> >> - { .procname = "net", },
> >> - { .procname = "appletalk", },
> >> - { }
> >> -};
> >> -
> >> static struct ctl_table_header *atalk_table_header;
> >>
> >> void atalk_register_sysctl(void)
> >> {
> >> + const struct ctl_path atalk_path[] = {
> > Shouldn't all of these be static const struct ?
> They needn't. And some variables are specified __net_initdata currently.
At least some objects are smaller with static.
$ size net/appletalk/sysctl_net_atalk.o.*
text data bss dec hex filename
324 236 48 608 260 net/appletalk/sysctl_net_atalk.o.withstatic
344 236 48 628 274 net/appletalk/sysctl_net_atalk.o.withoutstatic
^ permalink raw reply
* Re: [Bridge] [PATCH] net: make ctl_path local and const
From: Joe Perches @ 2010-10-20 3:28 UTC (permalink / raw)
To: Changli Gao
Cc: Andy Grover, James Morris, linux-sctp, rds-devel,
Pekka Savola (ipv6), linux-x25, dccp, bridge, Andrew, coreteam,
Arnaldo Carvalho de Melo, Alexey Kuznetsov, Joerg Reuter,
Sridhar Samudrala, Samuel Ortiz, Vlad Yasevich, netfilter,
Remi Denis-Courmont, linux-hams, Hideaki YOSHIFUJI, netdev,
linux-decnet-user, linux-kernel, Ralf Baechle, David S. Miller,
netfilter-devel, Hendry
In-Reply-To: <AANLkTi=pz4zYnnUSvb9FjVvCAURruHhJTz7pBKDi8Pdw@mail.gmail.com>
On Wed, 2010-10-20 at 11:10 +0800, Changli Gao wrote:
> On Wed, Oct 20, 2010 at 11:01 AM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2010-10-20 at 10:54 +0800, Changli Gao wrote:
> >> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> > []
> >> diff --git a/net/appletalk/sysctl_net_atalk.c b/net/appletalk/sysctl_net_atalk.c
> >> index 04e9c0d..b92f269 100644
> >> --- a/net/appletalk/sysctl_net_atalk.c
> >> +++ b/net/appletalk/sysctl_net_atalk.c
> >> @@ -42,16 +42,16 @@ static struct ctl_table atalk_table[] = {
> >> { },
> >> };
> >> -static struct ctl_path atalk_path[] = {
> >> - { .procname = "net", },
> >> - { .procname = "appletalk", },
> >> - { }
> >> -};
> >> -
> >> static struct ctl_table_header *atalk_table_header;
> >>
> >> void atalk_register_sysctl(void)
> >> {
> >> + const struct ctl_path atalk_path[] = {
> > Shouldn't all of these be static const struct ?
> They needn't. And some variables are specified __net_initdata currently.
At least some objects are smaller with static.
$ size net/appletalk/sysctl_net_atalk.o.*
text data bss dec hex filename
324 236 48 608 260 net/appletalk/sysctl_net_atalk.o.withstatic
344 236 48 628 274 net/appletalk/sysctl_net_atalk.o.withoutstatic
^ permalink raw reply
* Re: ib mad definitions
From: Jason Gunthorpe @ 2010-10-20 3:28 UTC (permalink / raw)
To: Ira Weiny
Cc: Hefty, Sean, Hal Rosenstock,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Sasha Khapyorsky
In-Reply-To: <20101019181256.d0a15afe.weiny2-i2BcT+NCU+M@public.gmane.org>
On Tue, Oct 19, 2010 at 06:12:56PM -0700, Ira Weiny wrote:
> I am all for "cleaner, more capable..." but why incompatible? If we want to
> start fresh and then convert OpenSM later, fine. But _don't_ forget to go
> back and convert OpenSM, because if you leave ib_types.h out there someone is
> going to use it and we are back to where we started... :-( Same for ibmad,
> when these definitions become available in umad, mad can be simplified.
ib_types.h should not be installed in /usr/include, stop doing that
and that risk goes away.
ibmad can't really be changed, it is system library with a defined
API. Maybe ibmad.2 or something, I don't know. I tried to use some of
the PR APIs in it, and I've found them not useful :(
For instance we can't just abandon the mad_get_fields approach because
we have real, usuable field access in umad, it has to stay.
> Right now there are 3 headers I find path record in.
> libibverbs: sa.h
This isn't a MAD path record, this is the kernel version, which is
unpacked. What we really needs is MAD 2 kernel and vice versa
conversion in a library. I already have code that does this in
several places :(
> libibmad: mad.h
You mean mad_get_fields IB_SA_PR_DGID_F, etc? It doesn't even have all
the fields :(
> opensm: ib_types.h
Yep.
> Node type is defined in:
>
> libibverbs: verbs.h
> opensm: ib_types.h
> libibmad: mad.h
>
> I could go on.
Keep in mind that for the most part libibmad is someones attempt to
make a set of accessors and structures for mads. It is incomplete. It
is largely unusable. I certainly haven't been able to use its PR
structure parsing functions for any real app. Was it just pulled out
of opensm? I don't know, I'd just as soon see that part of it be
discarded, and a complete set of structures added to umad.
opensm has unique problems because they want to remain independent of the
OFA stack, I don't think they have a choice but to duplicate.
Jason
--
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
^ permalink raw reply
* Re: [PATCH 11/25] ASoC: Samsung: Add common I2S driver
From: Jassi Brar @ 2010-10-20 3:27 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, kgene.kim, Jassi Brar, ben-linux, june.bae, lrg,
sw.youn
In-Reply-To: <20101020031333.GH3803@opensource.wolfsonmicro.com>
On Wed, Oct 20, 2010 at 12:13 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
>> struct s3c_audio_pdata is shared among I2S, AC97, SPDIF and PCM.
>> Here the idea is that controller drivers add specific structures to the union
>> as and when they need it.
>
> Right, what I'm saying is that it'd seem more natural to do things the
> other way around and embed the common structure within a device specific
> pdata structure instead of having the union.
Ok, will do it.
>> > Can we do this stuff with a variable storing the mask to use? It might
>> > compress the code a lot but I've not looked at what the bits actually
>> > are.
>
>> sorry, I am unable to understand what you suggest
>
> If we have something in the driver data struct specifying the masks to
> use then set these at probe time rather than having the if statements -
> probably the same mask can be used for playback & record if the
> bitfields are lined up similarly. This would then be:
>
> con |= data->active_mask;
> con &= ~data->pause_mask;
>
> or similar, possibly with some shifts.
Let me see if the space saved is worth the complication.
>> >> + /* Allow LRCLK-inverted version of the supported formats */
>> >> + switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
>> >> + case SND_SOC_DAIFMT_NB_NF:
>> >> + break;
>> >> + case SND_SOC_DAIFMT_NB_IF:
>> >> + if (tmp & MOD_LR_RLOW)
>> >> + tmp &= ~MOD_LR_RLOW;
>> >> + else
>> >> + tmp |= MOD_LR_RLOW;
>> >> + break;
>
>> > This looks a bit odd - it'll flip MOD_LR_FLOW if the frame is inverted,
>> > I'd expect it to want to set a specific value?
>
>> The frame polarity is specified by the Format(I2S, LSB, MSB), so we
>> set/clear MOD_LR_RLOW acc to the Format requested.
>> The Inversion request works relative to the Format -- if Frame inversion
>> is requested, we simply flip it from the value set during Format setting.
>
> Please add a comment explaining that you're inverting the orientation
> you set previously - it's really surprising when reading the code.
Ok, I'll add a comment.
Btw, isn't it the standard way? Don't other I2S CPU drivers do the same thing ?
>> ASoC machine drivers need to index the secondary dai that is
>> automatically created and registered by the cpu driver.
>> So, it needs to be available outside.
>> I'll make it SAMSUNG_I2S_SECOFF -- secondary offset of I2S(?)
>
> How would the index be used with multi-component?
For example, please look at [Patch 23/25]
+ /* Secondary is at offset MAX_I2S from Primary */
+ str = (char *)smdk_dai[SEC_PLAYBACK].cpu_dai_name;
+ str[strlen(str) - 1] = '0' + MAX_I2S;
Thanks.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply
* [PATCH][memcg+dirtylimit] Fix overwriting global vm dirty limit setting by memcg (Re: [PATCH v3 00/11] memcg: per cgroup dirty page accounting
From: KAMEZAWA Hiroyuki @ 2010-10-20 3:21 UTC (permalink / raw)
To: Greg Thelen
Cc: Andrew Morton, linux-kernel, linux-mm, containers, Andrea Righi,
Balbir Singh, Daisuke Nishimura, Minchan Kim, Ciju Rajan K,
David Rientjes
In-Reply-To: <1287448784-25684-1-git-send-email-gthelen@google.com>
One bug fix here.
==
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Now, at calculating dirty limit, vm_dirty_param() is called.
This function returns dirty-limit related parameters considering
memory cgroup settings.
Now, assume that vm_dirty_bytes=100M (global dirty limit) and
memory cgroup has 1G of pages and 40 dirty_ratio, dirtyable memory is
500MB.
In this case, global_dirty_limits will consider dirty_limt as
500 *0.4 = 200MB. This is bad...memory cgroup is not back door.
This patch limits the return value of vm_dirty_param() considring
global settings.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
include/linux/memcontrol.h | 4 ++--
mm/memcontrol.c | 29 ++++++++++++++++++++++++++++-
mm/page-writeback.c | 2 +-
3 files changed, 31 insertions(+), 4 deletions(-)
Index: dirty_limit_new/mm/memcontrol.c
===================================================================
--- dirty_limit_new.orig/mm/memcontrol.c
+++ dirty_limit_new/mm/memcontrol.c
@@ -1171,9 +1171,10 @@ static void __mem_cgroup_dirty_param(str
* can be moved after our access and writeback tends to take long time. At
* least, "memcg" will not be freed while holding rcu_read_lock().
*/
-void vm_dirty_param(struct vm_dirty_param *param)
+void vm_dirty_param(struct vm_dirty_param *param, unsigned long mem)
{
struct mem_cgroup *memcg;
+ u64 limit, bglimit;
if (mem_cgroup_disabled()) {
global_vm_dirty_param(param);
@@ -1183,6 +1184,32 @@ void vm_dirty_param(struct vm_dirty_para
rcu_read_lock();
memcg = mem_cgroup_from_task(current);
__mem_cgroup_dirty_param(param, memcg);
+ /*
+ * A limitation under memory cgroup is under global vm, too.
+ */
+ if (vm_dirty_ratio)
+ limit = mem * vm_dirty_ratio / 100;
+ else
+ limit = vm_dirty_bytes;
+ if (param->dirty_ratio) {
+ param->dirty_bytes = mem * param->dirty_ratio / 100;
+ param->dirty_ratio = 0;
+ }
+ if (param->dirty_bytes > limit)
+ param->dirty_bytes = limit;
+
+ if (dirty_background_ratio)
+ bglimit = mem * dirty_background_ratio / 100;
+ else
+ bglimit = dirty_background_bytes;
+
+ if (param->dirty_background_ratio) {
+ param->dirty_background_bytes =
+ mem * param->dirty_background_ratio /100;
+ param->dirty_background_ratio = 0;
+ }
+ if (param->dirty_background_bytes > bglimit)
+ param->dirty_background_bytes = bglimit;
rcu_read_unlock();
}
Index: dirty_limit_new/include/linux/memcontrol.h
===================================================================
--- dirty_limit_new.orig/include/linux/memcontrol.h
+++ dirty_limit_new/include/linux/memcontrol.h
@@ -171,7 +171,7 @@ static inline void mem_cgroup_dec_page_s
}
bool mem_cgroup_has_dirty_limit(void);
-void vm_dirty_param(struct vm_dirty_param *param);
+void vm_dirty_param(struct vm_dirty_param *param, u64 mem);
s64 mem_cgroup_page_stat(enum mem_cgroup_nr_pages_item item);
unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,
@@ -360,7 +360,7 @@ static inline bool mem_cgroup_has_dirty_
return false;
}
-static inline void vm_dirty_param(struct vm_dirty_param *param)
+static inline void vm_dirty_param(struct vm_dirty_param *param, u64 mem)
{
global_vm_dirty_param(param);
}
Index: dirty_limit_new/mm/page-writeback.c
===================================================================
--- dirty_limit_new.orig/mm/page-writeback.c
+++ dirty_limit_new/mm/page-writeback.c
@@ -466,7 +466,7 @@ void global_dirty_limits(unsigned long *
struct task_struct *tsk;
struct vm_dirty_param dirty_param;
- vm_dirty_param(&dirty_param);
+ vm_dirty_param(&dirty_param, avialable_memory);
if (dirty_param.dirty_bytes)
dirty = DIV_ROUND_UP(dirty_param.dirty_bytes, PAGE_SIZE);
^ permalink raw reply
* Re: oom_killer crash linux system
From: KOSAKI Motohiro @ 2010-10-20 3:24 UTC (permalink / raw)
To: Figo.zhang
Cc: kosaki.motohiro, Wu Fengguang, KAMEZAWA Hiroyuki, Minchan Kim,
linux-kernel@vger.kernel.org, rientjes@google.com, figo1802,
linux-mm@kvack.org
In-Reply-To: <1287543520.2074.1.camel@myhost>
>
> >
> > can you please try 1) invoke oom 2) get page-types -r again. I'm curious
> > that oom makes page accounting lost again. I mean, please send us oom
> > log and "page-types -r" result.
> >
> > thanks
>
> ok, i do the experiment and catch the log:
thanks.
> active_anon:398375 inactive_anon:82967 isolated_anon:0
> active_file:81 inactive_file:429 isolated_file:32
> unevictable:13 dirty:2 writeback:14 unstable:0
> free:11942 slab_reclaimable:2391 slab_unreclaimable:3303
> mapped:5617 shmem:33909 pagetables:2280 bounce:0
active_anon + inactive_anon + isolated_anon = 481342 pages ~= 1.8GB
Um, this oom doesn't makes accounting lost.
> here is the page-types log:
> flags page-count MB symbolic-flags long-symbolic-flags
>
> 0x0000000000005828 83024 324 ___U_l_____Ma_b___________________ uptodate,lru,mmap,anonymous,swapbacked
> 0x0000000000005868 358737 1401 ___U_lA____Ma_b___________________ uptodate,lru,active,mmap,anonymous,swapbacked
> total 515071 2011
page-types show similar result.
The big difference is, previous and current are showing some different processes.
only previous has VirtualBox, only current has vmware-usbarbit, etc..
Can you use same test environment?
--
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>
^ permalink raw reply
* Re: oom_killer crash linux system
From: KOSAKI Motohiro @ 2010-10-20 3:24 UTC (permalink / raw)
To: Figo.zhang
Cc: kosaki.motohiro, Wu Fengguang, KAMEZAWA Hiroyuki, Minchan Kim,
linux-kernel@vger.kernel.org, rientjes@google.com, figo1802,
linux-mm@kvack.org
In-Reply-To: <1287543520.2074.1.camel@myhost>
>
> >
> > can you please try 1) invoke oom 2) get page-types -r again. I'm curious
> > that oom makes page accounting lost again. I mean, please send us oom
> > log and "page-types -r" result.
> >
> > thanks
>
> ok, i do the experiment and catch the log:
thanks.
> active_anon:398375 inactive_anon:82967 isolated_anon:0
> active_file:81 inactive_file:429 isolated_file:32
> unevictable:13 dirty:2 writeback:14 unstable:0
> free:11942 slab_reclaimable:2391 slab_unreclaimable:3303
> mapped:5617 shmem:33909 pagetables:2280 bounce:0
active_anon + inactive_anon + isolated_anon = 481342 pages ~= 1.8GB
Um, this oom doesn't makes accounting lost.
> here is the page-types log:
> flags page-count MB symbolic-flags long-symbolic-flags
>
> 0x0000000000005828 83024 324 ___U_l_____Ma_b___________________ uptodate,lru,mmap,anonymous,swapbacked
> 0x0000000000005868 358737 1401 ___U_lA____Ma_b___________________ uptodate,lru,active,mmap,anonymous,swapbacked
> total 515071 2011
page-types show similar result.
The big difference is, previous and current are showing some different processes.
only previous has VirtualBox, only current has vmware-usbarbit, etc..
Can you use same test environment?
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: pacman @ 2010-10-20 3:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Segher Boessenkool, Mel Gorman, linux-mm, Andrew Morton,
linuxppc-dev, linux-kernel
In-Reply-To: <1287522168.2198.5.camel@pasglop>
Benjamin Herrenschmidt writes:
>
> On Tue, 2010-10-19 at 22:47 +0200, Segher Boessenkool wrote:
> >
> > It looks like it is the frame counter in an USB OHCI HCCA.
> > 16-bit, 1kHz update, offset x'80 in a page.
> >
> > So either the kernel forgot to call quiesce on it, or the firmware
> > doesn't implement that, or the firmware messed up some other way.
>
> I vote for the FW being on crack. Wouldn't be the first time with
> Pegasos.
>
> It's an OHCI or an UHCI in there ?
There's one of each... UHCI on the motherboard, OHCI on a card in a PCI
expansion slot. They shipped the ODW with the extra controller on an
expansion card since the on-board UHCI doesn't do USB2.0.
And that OHCI controller does appear to be the culprit. The 2 affected
addresses tick at 1000Hz until ohci-hcd is modprobe'd, then they stop.
I think the mm people can consider this closed. 6dda9d55 didn't do anything
but expose a problem which has been here all along. Will drop them from Cc
list in any further messages.
>
> Can you try in prom_init.c changing the prom_close_stdin() function to
> also close "stdout" ?
>
> if (prom_getprop(_prom->chosen, "stdin", &val, sizeof(val)) > 0)
> call_prom("close", 1, 0, val);
> + if (prom_getprop(_prom->chosen, "stdout", &val, sizeof(val)) > 0)
> + call_prom("close", 1, 0, val);
>
> See if that makes a difference ?
Huge difference. With no stdout to print to, the kernel seems to freeze up.
Or at least it loses the console. The last message it prints is "Device tree
struct 0x00933000 -> 0x00957000" then there's just nothing. I waited a while
for the console to come on but it didn't.
The diff fragment above applied inside prom_close_stdin, but there are some
prom_printf calls after prom_close_stdin. Calling prom_printf after closing
stdout sounds like it could be bad. If I moved it down below all the
prom_printf's, it would be after the "quiesce" call. Would that be acceptable
(or even interesting as an experiment)? Does a close need a quiesce after it?
--
Alan Curry
--
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>
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: pacman @ 2010-10-20 3:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Segher Boessenkool, Mel Gorman, linux-mm, Andrew Morton,
linuxppc-dev, linux-kernel
In-Reply-To: <1287522168.2198.5.camel@pasglop>
Benjamin Herrenschmidt writes:
>
> On Tue, 2010-10-19 at 22:47 +0200, Segher Boessenkool wrote:
> >
> > It looks like it is the frame counter in an USB OHCI HCCA.
> > 16-bit, 1kHz update, offset x'80 in a page.
> >
> > So either the kernel forgot to call quiesce on it, or the firmware
> > doesn't implement that, or the firmware messed up some other way.
>
> I vote for the FW being on crack. Wouldn't be the first time with
> Pegasos.
>
> It's an OHCI or an UHCI in there ?
There's one of each... UHCI on the motherboard, OHCI on a card in a PCI
expansion slot. They shipped the ODW with the extra controller on an
expansion card since the on-board UHCI doesn't do USB2.0.
And that OHCI controller does appear to be the culprit. The 2 affected
addresses tick at 1000Hz until ohci-hcd is modprobe'd, then they stop.
I think the mm people can consider this closed. 6dda9d55 didn't do anything
but expose a problem which has been here all along. Will drop them from Cc
list in any further messages.
>
> Can you try in prom_init.c changing the prom_close_stdin() function to
> also close "stdout" ?
>
> if (prom_getprop(_prom->chosen, "stdin", &val, sizeof(val)) > 0)
> call_prom("close", 1, 0, val);
> + if (prom_getprop(_prom->chosen, "stdout", &val, sizeof(val)) > 0)
> + call_prom("close", 1, 0, val);
>
> See if that makes a difference ?
Huge difference. With no stdout to print to, the kernel seems to freeze up.
Or at least it loses the console. The last message it prints is "Device tree
struct 0x00933000 -> 0x00957000" then there's just nothing. I waited a while
for the console to come on but it didn't.
The diff fragment above applied inside prom_close_stdin, but there are some
prom_printf calls after prom_close_stdin. Calling prom_printf after closing
stdout sounds like it could be bad. If I moved it down below all the
prom_printf's, it would be after the "quiesce" call. Would that be acceptable
(or even interesting as an experiment)? Does a close need a quiesce after it?
--
Alan Curry
^ permalink raw reply
* Re: PROBLEM: memory corrupting bug, bisected to 6dda9d55
From: pacman @ 2010-10-20 3:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Mel Gorman, linux-kernel, linux-mm, Andrew Morton, linuxppc-dev
In-Reply-To: <1287522168.2198.5.camel@pasglop>
Benjamin Herrenschmidt writes:
>
> On Tue, 2010-10-19 at 22:47 +0200, Segher Boessenkool wrote:
> >
> > It looks like it is the frame counter in an USB OHCI HCCA.
> > 16-bit, 1kHz update, offset x'80 in a page.
> >
> > So either the kernel forgot to call quiesce on it, or the firmware
> > doesn't implement that, or the firmware messed up some other way.
>
> I vote for the FW being on crack. Wouldn't be the first time with
> Pegasos.
>
> It's an OHCI or an UHCI in there ?
There's one of each... UHCI on the motherboard, OHCI on a card in a PCI
expansion slot. They shipped the ODW with the extra controller on an
expansion card since the on-board UHCI doesn't do USB2.0.
And that OHCI controller does appear to be the culprit. The 2 affected
addresses tick at 1000Hz until ohci-hcd is modprobe'd, then they stop.
I think the mm people can consider this closed. 6dda9d55 didn't do anything
but expose a problem which has been here all along. Will drop them from Cc
list in any further messages.
>
> Can you try in prom_init.c changing the prom_close_stdin() function to
> also close "stdout" ?
>
> if (prom_getprop(_prom->chosen, "stdin", &val, sizeof(val)) > 0)
> call_prom("close", 1, 0, val);
> + if (prom_getprop(_prom->chosen, "stdout", &val, sizeof(val)) > 0)
> + call_prom("close", 1, 0, val);
>
> See if that makes a difference ?
Huge difference. With no stdout to print to, the kernel seems to freeze up.
Or at least it loses the console. The last message it prints is "Device tree
struct 0x00933000 -> 0x00957000" then there's just nothing. I waited a while
for the console to come on but it didn't.
The diff fragment above applied inside prom_close_stdin, but there are some
prom_printf calls after prom_close_stdin. Calling prom_printf after closing
stdout sounds like it could be bad. If I moved it down below all the
prom_printf's, it would be after the "quiesce" call. Would that be acceptable
(or even interesting as an experiment)? Does a close need a quiesce after it?
--
Alan Curry
^ permalink raw reply
* [PATCH][memcg+dirtylimit] Fix overwriting global vm dirty limit setting by memcg (Re: [PATCH v3 00/11] memcg: per cgroup dirty page accounting
From: KAMEZAWA Hiroyuki @ 2010-10-20 3:21 UTC (permalink / raw)
To: Greg Thelen
Cc: Andrew Morton, linux-kernel, linux-mm, containers, Andrea Righi,
Balbir Singh, Daisuke Nishimura, Minchan Kim, Ciju Rajan K,
David Rientjes
In-Reply-To: <1287448784-25684-1-git-send-email-gthelen@google.com>
One bug fix here.
==
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Now, at calculating dirty limit, vm_dirty_param() is called.
This function returns dirty-limit related parameters considering
memory cgroup settings.
Now, assume that vm_dirty_bytes=100M (global dirty limit) and
memory cgroup has 1G of pages and 40 dirty_ratio, dirtyable memory is
500MB.
In this case, global_dirty_limits will consider dirty_limt as
500 *0.4 = 200MB. This is bad...memory cgroup is not back door.
This patch limits the return value of vm_dirty_param() considring
global settings.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
include/linux/memcontrol.h | 4 ++--
mm/memcontrol.c | 29 ++++++++++++++++++++++++++++-
mm/page-writeback.c | 2 +-
3 files changed, 31 insertions(+), 4 deletions(-)
Index: dirty_limit_new/mm/memcontrol.c
===================================================================
--- dirty_limit_new.orig/mm/memcontrol.c
+++ dirty_limit_new/mm/memcontrol.c
@@ -1171,9 +1171,10 @@ static void __mem_cgroup_dirty_param(str
* can be moved after our access and writeback tends to take long time. At
* least, "memcg" will not be freed while holding rcu_read_lock().
*/
-void vm_dirty_param(struct vm_dirty_param *param)
+void vm_dirty_param(struct vm_dirty_param *param, unsigned long mem)
{
struct mem_cgroup *memcg;
+ u64 limit, bglimit;
if (mem_cgroup_disabled()) {
global_vm_dirty_param(param);
@@ -1183,6 +1184,32 @@ void vm_dirty_param(struct vm_dirty_para
rcu_read_lock();
memcg = mem_cgroup_from_task(current);
__mem_cgroup_dirty_param(param, memcg);
+ /*
+ * A limitation under memory cgroup is under global vm, too.
+ */
+ if (vm_dirty_ratio)
+ limit = mem * vm_dirty_ratio / 100;
+ else
+ limit = vm_dirty_bytes;
+ if (param->dirty_ratio) {
+ param->dirty_bytes = mem * param->dirty_ratio / 100;
+ param->dirty_ratio = 0;
+ }
+ if (param->dirty_bytes > limit)
+ param->dirty_bytes = limit;
+
+ if (dirty_background_ratio)
+ bglimit = mem * dirty_background_ratio / 100;
+ else
+ bglimit = dirty_background_bytes;
+
+ if (param->dirty_background_ratio) {
+ param->dirty_background_bytes =
+ mem * param->dirty_background_ratio /100;
+ param->dirty_background_ratio = 0;
+ }
+ if (param->dirty_background_bytes > bglimit)
+ param->dirty_background_bytes = bglimit;
rcu_read_unlock();
}
Index: dirty_limit_new/include/linux/memcontrol.h
===================================================================
--- dirty_limit_new.orig/include/linux/memcontrol.h
+++ dirty_limit_new/include/linux/memcontrol.h
@@ -171,7 +171,7 @@ static inline void mem_cgroup_dec_page_s
}
bool mem_cgroup_has_dirty_limit(void);
-void vm_dirty_param(struct vm_dirty_param *param);
+void vm_dirty_param(struct vm_dirty_param *param, u64 mem);
s64 mem_cgroup_page_stat(enum mem_cgroup_nr_pages_item item);
unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,
@@ -360,7 +360,7 @@ static inline bool mem_cgroup_has_dirty_
return false;
}
-static inline void vm_dirty_param(struct vm_dirty_param *param)
+static inline void vm_dirty_param(struct vm_dirty_param *param, u64 mem)
{
global_vm_dirty_param(param);
}
Index: dirty_limit_new/mm/page-writeback.c
===================================================================
--- dirty_limit_new.orig/mm/page-writeback.c
+++ dirty_limit_new/mm/page-writeback.c
@@ -466,7 +466,7 @@ void global_dirty_limits(unsigned long *
struct task_struct *tsk;
struct vm_dirty_param dirty_param;
- vm_dirty_param(&dirty_param);
+ vm_dirty_param(&dirty_param, avialable_memory);
if (dirty_param.dirty_bytes)
dirty = DIV_ROUND_UP(dirty_param.dirty_bytes, PAGE_SIZE);
--
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>
^ permalink raw reply
* [U-Boot] Data bus error
From: sywang @ 2010-10-20 3:20 UTC (permalink / raw)
To: u-boot
In-Reply-To: <20101019222635.56FC71359B7@gemini.denx.de>
Did you have the data bus error with CN5010-based board at porting u-boot?
How to debug? Any suggestions on this?
Thanks!
Shuyou
^ permalink raw reply
* Re: [patch 31/35] fs: icache per-zone inode LRU
From: Nick Piggin @ 2010-10-20 3:20 UTC (permalink / raw)
To: KOSAKI Motohiro; +Cc: Dave Chinner, npiggin, linux-kernel, linux-fsdevel
In-Reply-To: <20101020115432.1821.A69D9226@jp.fujitsu.com>
On Wed, Oct 20, 2010 at 12:14:32PM +0900, KOSAKI Motohiro wrote:
> > On Tue, Oct 19, 2010 at 02:42:47PM +1100, npiggin@kernel.dk wrote:
> > Anyway, my main point is that tying the LRU and shrinker scaling to
> > the implementation of the VM is a one-off solution that doesn't work
> > for generic infrastructure. Other subsystems need the same
> > large-machine scaling treatment, and there's no way we should be
> > tying them all into the struct zone. It needs further abstraction.
>
> I'm not sure what data structure is best. I can only say current
> zone unawareness slab shrinker might makes following sad scenario.
>
> o DMA zone shortage invoke and plenty icache in NORMAL zone dropping
> o NUMA aware system enable zone_reclaim_mode, but shrink_slab() still
> drop unrelated zone's icache
>
> both makes performance degression. In other words, Linux does not have
> flat memory model. so, I don't think Nick's basic concept is wrong.
> It's straight forward enhancement. but if it don't fit current shrinkers,
> I'd like to discuss how to make better data structure.
>
>
>
> and I have dump question (sorry, I don't know xfs at all). current
> xfs_mount is below.
>
> typedef struct xfs_mount {
> ...
> struct shrinker m_inode_shrink; /* inode reclaim shrinker */
> } xfs_mount_t;
>
>
> Do you mean xfs can't convert shrinker to shrinker[ZONES]? If so, why?
Well if XFS were to use per-ZONE shrinkers, it would remain with a
single shrinker context per-sb like it has now, but it would divide
its object management into per-zone structures.
For subsystems that aren't important, don't take much memory or have
much reclaim throughput, they are free to ignore the zone argument
and keep using the global input to the shrinker.
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/3] Introduce threadlets
From: Venkateswararao Jujjuri (JV) @ 2010-10-20 3:19 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Arun R Bharadwaj, qemu-devel, balbir
In-Reply-To: <4CBE0F5F.3020204@codemonkey.ws>
On 10/19/2010 2:36 PM, Anthony Liguori wrote:
> On 10/19/2010 01:36 PM, Balbir Singh wrote:
>>> + qemu_mutex_lock(&(queue->lock));
>>> + while (1) {
>>> + ThreadletWork *work;
>>> + int ret = 0;
>>> +
>>> + while (QTAILQ_EMPTY(&(queue->request_list))&&
>>> + (ret != ETIMEDOUT)) {
>>> + ret = qemu_cond_timedwait(&(queue->cond),
>>> + &(queue->lock), 10*100000);
>>>
>> Ewww... what is 10*100000, can we use something more meaningful
>> please?
>>
>
> A define is fine but honestly, it's pretty darn obvious what it means...
>
>>> + }
>>> +
>>> + assert(queue->idle_threads != 0);
>>>
>> This assertion holds because we believe one of the idle_threads
>> actually did the dequeuing, right?
>>
>
> An idle thread is a thread is one that is not doing work. At this point in the
> code, we are not doing any work (yet) so if idle_threads count is zero,
> something is horribly wrong. We're also going to unconditionally decrement in
> the future code path which means that if idle_threads is 0, it's going to become
> -1.
>
> The use of idle_thread is to detect whether it's necessary to spawn an
> additional thread.
>
>>> + if (QTAILQ_EMPTY(&(queue->request_list))) {
>>> + if (queue->cur_threads> queue->min_threads) {
>>> + /* We retain the minimum number of threads */
>>> + break;
>>> + }
>>> + } else {
>>> + work = QTAILQ_FIRST(&(queue->request_list));
>>> + QTAILQ_REMOVE(&(queue->request_list), work, node);
>>> +
>>> + queue->idle_threads--;
>>> + qemu_mutex_unlock(&(queue->lock));
>>> +
>>> + /* execute the work function */
>>> + work->func(work);
>>> +
>>> + qemu_mutex_lock(&(queue->lock));
>>> + queue->idle_threads++;
>>> + }
>>> + }
>>> +
>>> + queue->idle_threads--;
>>> + queue->cur_threads--;
>>> + qemu_mutex_unlock(&(queue->lock));
>>> +
>>> + return NULL;
>>>
>> Does anybody do a join on the exiting thread from the pool?
>>
>
> No. The thread is created in a detached state.
>
>>> +}
>>> +
>>> +static void spawn_threadlet(ThreadletQueue *queue)
>>> +{
>>> + QemuThread thread;
>>> +
>>> + queue->cur_threads++;
>>> + queue->idle_threads++;
>>> +
>>> + qemu_thread_create(&thread, threadlet_worker, queue);
>>>
>>
>>> +}
>>> +
>>> +/**
>>> + * submit_threadletwork_to_queue: Submit a new task to a private queue to be
>>> + * executed asynchronously.
>>> + * @queue: Per-subsystem private queue to which the new task needs
>>> + * to be submitted.
>>> + * @work: Contains information about the task that needs to be submitted.
>>> + */
>>> +void submit_threadletwork_to_queue(ThreadletQueue *queue, ThreadletWork *work)
>>> +{
>>> + qemu_mutex_lock(&(queue->lock));
>>> + if (queue->idle_threads == 0&& queue->cur_threads< queue->max_threads) {
>>> + spawn_threadlet(queue);
>>>
>> So we hold queue->lock, spawn the thread, the spawned thread tries to
>> acquire queue->lock
>>
>
> Yup.
>
>>> + }
>>> + QTAILQ_INSERT_TAIL(&(queue->request_list), work, node);
>>> + qemu_mutex_unlock(&(queue->lock));
>>> + qemu_cond_signal(&(queue->cond));
>>>
>> In the case that we just spawned the threadlet, the cond_signal is
>> spurious. If we need predictable scheduling behaviour,
>> qemu_cond_signal needs to happen with queue->lock held.
>>
>
> It doesn't really affect predictability..
>
>> I'd rewrite the function as
>>
>> /**
>> * submit_threadletwork_to_queue: Submit a new task to a private queue to be
>> * executed asynchronously.
>> * @queue: Per-subsystem private queue to which the new task needs
>> * to be submitted.
>> * @work: Contains information about the task that needs to be submitted.
>> */
>> void submit_threadletwork_to_queue(ThreadletQueue *queue, ThreadletWork *work)
>> {
>> qemu_mutex_lock(&(queue->lock));
>> if (queue->idle_threads == 0&& (queue->cur_threads< queue->max_threads)) {
>> spawn_threadlet(queue);
>> } else {
>> qemu_cond_signal(&(queue->cond));
>> }
>> QTAILQ_INSERT_TAIL(&(queue->request_list), work, node);
>> qemu_mutex_unlock(&(queue->lock));
>> }
>>
>
> I think this is a lot more fragile. You're relying on the fact that signal will
> not cause the signalled thread to actually awaken until we release the lock and
> doing work after signalling that the signalled thread needs to be completed
> before it wakes up.
Given that qemu_cond_timedwait() need to get the queue->lock before returning
the singalled thread will wakeup and wait on the queue->lock.
- JV
>
> I think you're a lot more robust in the long term if you treat condition
> signalling as a hand off point because it makes the code a lot more explicit
> about what's happening.
>
>>> +/**
>>> + * submit_threadletwork: Submit to the global queue a new task to be executed
>>> + * asynchronously.
>>> + * @work: Contains information about the task that needs to be submitted.
>>> + */
>>> +void submit_threadletwork(ThreadletWork *work)
>>> +{
>>> + if (unlikely(!globalqueue_init)) {
>>> + threadlet_queue_init(&globalqueue, MAX_GLOBAL_THREADS,
>>> + MIN_GLOBAL_THREADS);
>>> + globalqueue_init = 1;
>>> + }
>>>
>> What protects globalqueue_init?
>>
>
> qemu_mutex, and that unlikely is almost certainly a premature optimization.
>
> Regards,
>
> Anthony Liguori
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
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.